From f4f2ffb59918febe163cc455bc2d8494956716ad Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 23 Jan 2026 09:58:27 +0000 Subject: [PATCH 01/18] function shell --- .idea/copilot.data.migration.agent.xml | 6 ++++++ recommendations/RoofRecommendations.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 .idea/copilot.data.migration.agent.xml diff --git a/.idea/copilot.data.migration.agent.xml b/.idea/copilot.data.migration.agent.xml new file mode 100644 index 00000000..4ea72a91 --- /dev/null +++ b/.idea/copilot.data.migration.agent.xml @@ -0,0 +1,6 @@ + + + + + \ No newline at end of file diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 1e5636ff..7f7c334e 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -119,6 +119,24 @@ class RoofRecommendations: return (full_insulated_room_roof or room_roof_insulated_at_rafters) and not has_non_invasive_recommendation + def recommend_sloping_ceiling(self): + """ + Sloping ceiling insulation recommendations are different from other roof types, though + the description of the roof appears to be quite similar to a roof with a loft. In order to + deduce the roof type, we apply the following logic: + + 1) If the roof is descrbed as pitched, insulated, without a loft insulation thickness, it's + an insulated sloped ceiling + 2) If the roof insulation is assumed, it implies that the surveyor could not gain access to the + roof and therefore it's a loft + 3) If it's a pitched roof that is uninsulated and is NOT assumed, and there is not loft insulation + recommendation, this implies that the surveyor was able to gain access to the roof and there was no + loft insulation recommendation so it must be a sloping ceiling since loft insulation is a default + recommendation for an uninsualted loft + :return: + """ + pass + def recommend(self, phase, measures=None, default_u_values=False): if self.property.roof["has_dwelling_above"]: From 0ad3f099026854c4cc28d3040d3fcee3bee68ef8 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 26 Jan 2026 12:25:51 +0000 Subject: [PATCH 02/18] refactoring roof recommendations logic --- asset_list/app.py | 10 +- etl/find_my_epc/RetrieveFindMyEpc.py | 2 +- recommendations/Costs.py | 31 +++- recommendations/RoofRecommendations.py | 154 +++++++++++++++--- .../tests/test_roof_recommendations.py | 40 +++++ 5 files changed, 199 insertions(+), 38 deletions(-) diff --git a/asset_list/app.py b/asset_list/app.py index 21a06a07..01906c5f 100644 --- a/asset_list/app.py +++ b/asset_list/app.py @@ -60,7 +60,7 @@ def app(): """ data_folder = "/Users/khalimconn-kowlessar/Documents/hestia/Customers/Hackney" - data_filename = "Domna SHF Wave 3.xlsx" + data_filename = "Domna SHF Wave 3 (3).xlsx" sheet_name = "Domna Wave 3" postcode_column = 'Postcode' address1_column = "Address 1" @@ -68,11 +68,11 @@ def app(): fulladdress_column = None address_cols_to_concat = ["Address 1"] missing_postcodes_method = None - landlord_year_built = None + landlord_year_built = "Construction Years" landlord_os_uprn = "UPRN" - landlord_property_type = None - landlord_built_form = None - landlord_wall_construction = None + landlord_property_type = "Type" + landlord_built_form = "Attachment" + landlord_wall_construction = "Wall type" landlord_roof_construction = None landlord_heating_system = None landlord_existing_pv = None diff --git a/etl/find_my_epc/RetrieveFindMyEpc.py b/etl/find_my_epc/RetrieveFindMyEpc.py index cf6659f9..82215443 100644 --- a/etl/find_my_epc/RetrieveFindMyEpc.py +++ b/etl/find_my_epc/RetrieveFindMyEpc.py @@ -665,7 +665,7 @@ class RetrieveFindMyEpc: ], "Change heating to gas condensing boiler": ["boiler_upgrade"], "Fan assisted storage heaters and dual immersion cylinder": ["high_heat_retention_storage_heaters"], - "Flat roof or sloping ceiling insulation": ["flat_roof_insulation"], + "Flat roof or sloping ceiling insulation": ["flat_roof_insulation", "sloping_ceiling_insulation"], "Heating controls (room thermostat)": [ "roomstat_programmer_trvs", "time_temperature_zone_control" ], diff --git a/recommendations/Costs.py b/recommendations/Costs.py index 60b1d8a2..3a65312e 100644 --- a/recommendations/Costs.py +++ b/recommendations/Costs.py @@ -160,6 +160,13 @@ class Costs: "low_energy_lighting": 0.26, "high_heat_retention_storage_heaters": 0.1, "windows_glazing": 0.15, + "boiler_upgrade": 0.26, + "time_and_temperature_zone_control": 0.1, + "roomstat_programmer_trvs": 0.1, + "room_roof_insulation": 0.26, + "heater_removal": 0.1, + "sealing_open_fireplace": 0.1, + "mechanical_ventilation": 0.26 } # Preliminaries are a percentage of the total cost of the work and covers the cost of site-specific costs @@ -664,10 +671,12 @@ class Costs: subtotal_before_vat = total_cost / (1 + self.VAT_RATE) vat = total_cost - subtotal_before_vat + contingency_rate = self.CONTINGENCIES["roomstat_programmer_trvs"] + return { "total": total_cost, - "contingency": total_cost * self.CONTINGENCY, - "contingency_rate": self.CONTINGENCY, + "contingency": total_cost * contingency_rate, + "contingency_rate": contingency_rate, "subtotal": subtotal_before_vat, "vat": vat, "labour_hours": labour_hours, @@ -698,10 +707,12 @@ class Costs: labour_days = np.ceil(labour_hours / 8) + contingency_rate = self.CONTINGENCIES["time_and_temperature_zone_control"] + return { "total": total_cost, - "contingency": total_cost * self.CONTINGENCY, - "contingency_rate": self.CONTINGENCY, + "contingency": total_cost * contingency_rate, + "contingency_rate": contingency_rate, "subtotal": subtotal_before_vat, "vat": vat, "labour_hours": labour_hours, @@ -752,10 +763,12 @@ class Costs: subtotal_before_vat = removal_cost total_cost = subtotal_before_vat + vat + contingency_rate = self.CONTINGENCIES["heater_removal"] + return { "total": total_cost, - "contingency": total_cost * self.CONTINGENCY, - "contingency_rate": self.CONTINGENCY, + "contingency": total_cost * contingency_rate, + "contingency_rate": contingency_rate, "subtotal": subtotal_before_vat, "vat": vat, "labour_hours": removal_labour_hours, @@ -858,10 +871,12 @@ class Costs: subtotal_before_vat += system_change_cost_before_vat vat += system_change_vat + contingency_rate = self.CONTINGENCIES["boiler_upgrade"] + return { "total": total_cost, - "contingency": total_cost * self.CONTINGENCY, - "contingency_rate": self.CONTINGENCY, + "contingency": total_cost * contingency_rate, + "contingency_rate": contingency_rate, "subtotal": subtotal_before_vat, "vat": vat, "labour_hours": labour_hours, diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 7f7c334e..1d6fe06c 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -137,41 +137,127 @@ class RoofRecommendations: """ pass - def recommend(self, phase, measures=None, default_u_values=False): + @staticmethod + def is_sloping_ceiling_appropriate( + is_pitched: bool, is_loft: bool, is_assumed: bool, has_sloping_ceiling_recommendation: bool, + primary_roof_is_sloped: bool + ) -> bool: + """ + :param is_pitched: Boolean - indicates whether or not the roof is pitched + :param is_loft: Boolean - indicates whether or not the roof is described as a loft + :param is_assumed: Boolean - indiates if the assessment of the roof is assumed or actually confirmed + :param has_sloping_ceiling_recommendation: Boolean - indicates if the property has a sloping ceiling + recommendation + :param primary_roof_is_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to + an extension) + :return: + """ + # We need to check: + # 1) If the property has a pitched roof + # 2) Does it have a recommendation for sloping ceiling + # 3) Is the insulation status NOT assumed + # 4) Is there a sloping ceiling recommendation (this may relate to the primary or secondary roof) + + # If we have a loft primary roof and sloping cei + + # The property is pitched, not a loft, not assumed and has a sloping ceiling rec + if (is_pitched and not is_loft and not is_assumed and has_sloping_ceiling_recommendation and + primary_roof_is_sloped): + return True + + return False + + @staticmethod + def is_loft_insulation_appropriate( + non_invasive_recommendations, measures, is_pitched, is_at_rafters, rir_over_loft + ) -> bool: + """ + Determine if loft insulation is appropriate + :param non_invasive_recommendations: List - list of non-invasive recommendations + :param measures: List - list of measures + :param is_pitched: Boolean - indicates whether or not the roof is pitched + :param is_at_rafters: Boolean - indicates whether or not the loft insulation is at rafters + :param rir_over_loft: Boolean - indicates whether or not there we should be doing RIR insulation + :return: + """ + + has_li_in_measures = "loft_insulation" in measures + has_li_non_invasive_recommendation = any( + x["type"] == "loft_insulation" for x in non_invasive_recommendations + ) + + return has_li_non_invasive_recommendation or ( + is_pitched and has_li_in_measures and not is_at_rafters + ) and not rir_over_loft + + @staticmethod + def is_flat_roof_insulation_appropriate( + is_flat: bool, measures: List, non_invasive_recommendations: List + ) -> bool: + """ + Determine if flat roof insulation is appropriate + :param is_flat: Boolean - indicates whether or not the roof is flat + :param measures: List - list of measures + :param non_invasive_recommendations: List - list of non-invasive recommendations + :return: + """ + + flat_roof_in_measures = "flat_roof_insulation" in measures + flat_roof_non_invasive_rec = has_li_non_invasive_recommendation = any( + x["type"] == "flat_roof_insulation" for x in non_invasive_recommendations + ) + + return (is_flat and flat_roof_in_measures) or flat_roof_non_invasive_rec + + def _does_roof_need_recommendation(self, measures: List | None = None, u_value: float | None = None): + """ + Utility function to recommend which contains the logic to determine whether the roof needs a recommendation + :return: + """ + # If there is a property above, nothing can be done if self.property.roof["has_dwelling_above"]: - return + return False - measures = MEASURE_MAP["roof_insulation"] if measures is None else measures - - u_value = self.property.roof["thermal_transmittance"] - - # If we have a flat roof but we don't have flat roof as a measure, we exit + # If we have a flat roof but not flat roof insulation recommendation if self.property.roof["is_flat"] and "flat_roof_insulation" not in measures: - return + return False - # We check if the roof is already insulated and if so, we exit - - # Building regulations part L recommend installing at least 270mm of insulation, however generally we - # experience diminishing returns in terms of SAP once we go beyond around 150mm of insulation - # This only holds true for pitched roofs. + # Logic to check if we have an already insulated loft if self.is_loft_already_insulated(measures): - return + return False + # Logic to check if we have an insulated flat roof if (self.insulation_thickness >= self.MINIMUM_FLAT_ROOF_ISULATION_MM) and self.property.roof["is_flat"]: - return + return False + # Logic to check if we have an already insulated room in roof if self.is_room_roof_insulated_or_unsuitable(measures): - return + return False if self.property.roof["is_thatched"]: - return + return False - # If we have a u-value and we don't have a non-invasive recommendation, we can't recommend anything if (u_value is not None) and not any( x in MEASURE_MAP["roof_insulation"] for x in [r["type"] for r in self.property.non_invasive_recommendations] ): - # We don't have enough information to provide a recommendation + return False + + def recommend(self, phase: int, measures: List | None = None, default_u_values: bool = False): + """ + Main method to recommend roof insulation measures + :param phase: Integer - phase of the recommendation, determines the order in which recommendations are + applied to the property + :param measures: List - list of measures to consider for recommendation + :param default_u_values: Boolean - whether or not to use default u-values for the recommendations + :return: + """ + + measures = MEASURE_MAP["roof_insulation"] if measures is None else measures + u_value = self.property.roof["thermal_transmittance"] + property_needs_roof_recommendation = self._does_roof_need_recommendation(measures, u_value) + + if not property_needs_roof_recommendation: return u_value = get_roof_u_value( @@ -200,17 +286,37 @@ class RoofRecommendations: # 1) We have an uninsulated loft (assumed) # 2) We have a non-intrusive recommendation for room in roof insulation + is_pitched = self.property.roof["is_pitched"] + is_loft = self.property.roof["is_loft"] + is_assumed = self.property.roof["is_assumed"] + is_at_rafters = self.property.roof["is_at_rafters"] + has_sloping_ceiling_recommendation = any( + x["type"] == "sloping_ceiling_insulation" for x in non_invasive_recommendations + ) + primary_roof_is_sloped = False # TODO + rir_over_loft = ( - self.property.roof["is_pitched"] and + is_pitched and self.property.roof["insulation_thickness"] == "none" and "room_in_roof_insulation" in [x["type"] for x in non_invasive_recommendations] ) + needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( + is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed, + has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, + primary_roof_is_sloped=primary_roof_is_sloped + ) + + needs_loft_insulation = self.is_loft_insulation_appropriate( + non_invasive_recommendations=non_invasive_recommendations, measures=measures, + is_pitched=is_pitched, is_at_rafters=is_at_rafters, rir_over_loft=rir_over_loft + ) + + ################################################## + # ~~~~~ Loft Insulation Recommendation Logic ~~~~~ + ################################################## # We firstly handle non-intrusive recommendations, which may override the normal roof insulation recommendations - if ("loft_insulation" in [x["type"] for x in non_invasive_recommendations]) or ( - self.property.roof["is_pitched"] and "loft_insulation" in measures and - not self.property.roof["is_at_rafters"] - ) and not rir_over_loft: + if needs_loft_insulation: self.recommend_roof_insulation( u_value=u_value, insulation_thickness=self.insulation_thickness, diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index 2241aeb7..b8cea10b 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -2,6 +2,7 @@ from backend.Property import Property from recommendations.RoofRecommendations import RoofRecommendations from recommendations.tests.test_data.materials import materials from etl.epc.Record import EPCRecord +import pytest class TestRoofRecommendations: @@ -402,3 +403,42 @@ class TestRoofRecommendations: roof_recommender14.recommend(phase=0) assert not roof_recommender14.recommendations + + # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ + @pytest.mark.parameterize("roof", + [ + ( + # For this example, the roof is pitched, without insulation and the description + # isn't assumed + {'original_description': 'Pitched, no insulation', 'thermal_transmittance': None, + 'thermal_transmittance_unit': None, + 'is_pitched': True, 'is_roof_room': False, 'is_loft': False, 'is_flat': False, + 'is_thatched': False, + 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, + 'is_valid': True, + 'insulation_thickness': 'none'} + ) + ] + ) + def test_sloping_ceiling_valid(self, roof): + # All conditions are met and therefore we should produce a sloping ceiling recommendation + assert RoofRecommendations.is_sloping_ceiling_appropriate( + is_pitched=True, is_loft=False, is_assumed=False, has_sloping_ceiling_recommendation=True + ) + + # One condition not met - we cannot verify + assert not RoofRecommendations.is_sloping_ceiling_appropriate( + is_pitched=True, is_loft=True, is_assumed=False, has_sloping_ceiling_recommendation=True + ) + assert not RoofRecommendations.is_sloping_ceiling_appropriate( + is_pitched=False, is_loft=False, is_assumed=False, has_sloping_ceiling_recommendation=True, + primary_roof_is_sloped=True + ) + assert not RoofRecommendations.is_sloping_ceiling_appropriate( + is_pitched=True, is_loft=False, is_assumed=True, has_sloping_ceiling_recommendation=True, + primary_roof_is_sloped=True + ) + assert not RoofRecommendations.is_sloping_ceiling_appropriate( + is_pitched=True, is_loft=False, is_assumed=True, has_sloping_ceiling_recommendation=True, + primary_roof_is_sloped=True + ) From e8b7a569ff2daca125fd830da926575704fe66e9 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 26 Jan 2026 13:25:15 +0000 Subject: [PATCH 03/18] working on rir insulation| --- .idea/copilot.data.migration.ask.xml | 6 ++ .idea/copilot.data.migration.edit.xml | 6 ++ recommendations/RoofRecommendations.py | 113 +++++++++++++++++++------ 3 files changed, 100 insertions(+), 25 deletions(-) create mode 100644 .idea/copilot.data.migration.ask.xml create mode 100644 .idea/copilot.data.migration.edit.xml diff --git a/.idea/copilot.data.migration.ask.xml b/.idea/copilot.data.migration.ask.xml new file mode 100644 index 00000000..7ef04e2e --- /dev/null +++ b/.idea/copilot.data.migration.ask.xml @@ -0,0 +1,6 @@ + + + + + \ No newline at end of file diff --git a/.idea/copilot.data.migration.edit.xml b/.idea/copilot.data.migration.edit.xml new file mode 100644 index 00000000..8648f940 --- /dev/null +++ b/.idea/copilot.data.migration.edit.xml @@ -0,0 +1,6 @@ + + + + + \ No newline at end of file diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 1d6fe06c..6625aeb0 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -170,7 +170,13 @@ class RoofRecommendations: @staticmethod def is_loft_insulation_appropriate( - non_invasive_recommendations, measures, is_pitched, is_at_rafters, rir_over_loft + measures: List, + is_pitched: bool, + is_at_rafters: bool, + rir_over_loft: bool, + is_assumed: bool, + has_loft_insulation_recommendation: bool, + has_sloping_ceiling_recommendation: bool ) -> bool: """ Determine if loft insulation is appropriate @@ -179,36 +185,59 @@ class RoofRecommendations: :param is_pitched: Boolean - indicates whether or not the roof is pitched :param is_at_rafters: Boolean - indicates whether or not the loft insulation is at rafters :param rir_over_loft: Boolean - indicates whether or not there we should be doing RIR insulation + :param is_assumed: Boolean - indicates whether or not the roof insulation status is assumed + :param has_loft_insulation_recommendation: Boolean - indicates whether or not there + is a loft insulation non-invasive recommendation + :param has_sloping_ceiling_recommendation: Boolean - indicates whether or not there + is a sloping ceiling non-invasive recommendation :return: """ has_li_in_measures = "loft_insulation" in measures - has_li_non_invasive_recommendation = any( - x["type"] == "loft_insulation" for x in non_invasive_recommendations - ) - return has_li_non_invasive_recommendation or ( + # Key business logic: + # If we have a pitched roof, no insulation, it's not assumed and we have a sloping ceiling recommendation, + # we do NOT recommend loft insulation + if is_pitched and not is_assumed and has_sloping_ceiling_recommendation: + return False + + return has_loft_insulation_recommendation or ( is_pitched and has_li_in_measures and not is_at_rafters ) and not rir_over_loft @staticmethod def is_flat_roof_insulation_appropriate( - is_flat: bool, measures: List, non_invasive_recommendations: List + is_flat: bool, measures: List, has_flat_roof_recommendation: bool ) -> bool: """ Determine if flat roof insulation is appropriate :param is_flat: Boolean - indicates whether or not the roof is flat :param measures: List - list of measures - :param non_invasive_recommendations: List - list of non-invasive recommendations - :return: + :param has_flat_roof_recommendation: Boolean - indicates whether or not there is a flat roof non-invasive + recommendation + :return: Boolean """ flat_roof_in_measures = "flat_roof_insulation" in measures - flat_roof_non_invasive_rec = has_li_non_invasive_recommendation = any( - x["type"] == "flat_roof_insulation" for x in non_invasive_recommendations - ) - return (is_flat and flat_roof_in_measures) or flat_roof_non_invasive_rec + return (is_flat and flat_roof_in_measures) or has_flat_roof_recommendation + + @staticmethod + def is_room_roof_insulation_appropriate( + is_room_roof, measures, rir_over_loft, has_room_roof_recommendation + ): + """ + Determine if room roof insulation is appropriate + :param is_room_roof: Boolean - indicates whether or not the roof is a room roof + :param measures: List - list of measures + :param rir_over_loft: Boolean - indicates whether or not there we should be doing RIR insulation + :param has_room_roof_recommendation: Boolean - indicates whether or not there is a room roof non-invasive + recommendation + :return: + """ + return is_room_roof and ("room_roof_insulation" in measures) or ( + has_room_roof_recommendation or rir_over_loft + ) def _does_roof_need_recommendation(self, measures: List | None = None, u_value: float | None = None): """ @@ -243,6 +272,28 @@ class RoofRecommendations: ): return False + @staticmethod + def _is_primary_roof_sloped( + is_pitched: bool, is_loft: bool, is_assumed: bool + ): + """ + Determine if the primary roof is sloped + :param is_pitched: bool - is the roof pitched + :param is_loft: bool - is the roof a loft + :param is_assumed: bool - is the roof insulation status assumed + :return: + """ + # Conditions for this to be true + # Case 1 + # In the property roof description (primary roof) + # 1) Pitched Roof + # 2) Uninsulated + # 3) Not assumed + if is_pitched and not is_loft and not is_assumed: + return True + + return False + def recommend(self, phase: int, measures: List | None = None, default_u_values: bool = False): """ Main method to recommend roof insulation measures @@ -290,14 +341,21 @@ class RoofRecommendations: is_loft = self.property.roof["is_loft"] is_assumed = self.property.roof["is_assumed"] is_at_rafters = self.property.roof["is_at_rafters"] + is_flat = self.property.roof["is_flat"] + is_room_roof = self.property.roof["is_roof_room"] + has_sloping_ceiling_recommendation = any( x["type"] == "sloping_ceiling_insulation" for x in non_invasive_recommendations ) - primary_roof_is_sloped = False # TODO + has_loft_insulation_recommendation = any(x["type"] == "loft_insulation" for x in non_invasive_recommendations) + has_flat_roof_recommendation = any(x["type"] == "flat_roof_insulation" for x in non_invasive_recommendations) + has_room_roof_recommendation = any(x["type"] == "room_roof_insulation" for x in non_invasive_recommendations) + primary_roof_is_sloped = self._is_primary_roof_sloped( + is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed + ) rir_over_loft = ( - is_pitched and - self.property.roof["insulation_thickness"] == "none" and + is_pitched and self.property.roof["insulation_thickness"] == "none" and "room_in_roof_insulation" in [x["type"] for x in non_invasive_recommendations] ) @@ -308,8 +366,19 @@ class RoofRecommendations: ) needs_loft_insulation = self.is_loft_insulation_appropriate( - non_invasive_recommendations=non_invasive_recommendations, measures=measures, - is_pitched=is_pitched, is_at_rafters=is_at_rafters, rir_over_loft=rir_over_loft + measures=measures, is_pitched=is_pitched, is_at_rafters=is_at_rafters, + rir_over_loft=rir_over_loft, has_loft_insulation_recommendation=has_loft_insulation_recommendation, + is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation + ) + + needs_flat_roof_insulation = self.is_flat_roof_insulation_appropriate( + is_flat=is_flat, measures=measures, has_flat_roof_recommendation=has_flat_roof_recommendation + ) + needs_rir_insulation = self.is_room_roof_insulation_appropriate( + is_room_roof=is_room_roof, + measures=measures, + rir_over_loft=rir_over_loft, + has_room_roof_recommendation=has_room_roof_recommendation ) ################################################## @@ -327,10 +396,7 @@ class RoofRecommendations: ) return - if ( - (self.property.roof["is_flat"] and "flat_roof_insulation" in measures) or - "flat_roof_insulation" in [x["type"] for x in non_invasive_recommendations] - ): + if needs_flat_roof_insulation: self.recommend_roof_insulation( u_value=u_value, insulation_thickness=0, @@ -343,10 +409,7 @@ class RoofRecommendations: # There are cases where the property might have a room roof as the second roof, but we have a recommendation for # it, so we allow this override - if self.property.roof["is_roof_room"] and ("room_roof_insulation" in measures) or ( - "room_roof_insulation" in [x["type"] for x in non_invasive_recommendations] or - rir_over_loft - ): + if needs_rir_insulation: self.recommend_room_roof_insulation(u_value, phase, default_u_values) return From 64eb2e2f204ee6f8d50dc7f4adb7646c23d4e6ff Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 26 Jan 2026 17:49:56 +0000 Subject: [PATCH 04/18] added in basic process for sloping ceiling --- backend/app/plan/schemas.py | 10 +++- recommendations/Costs.py | 68 ++++++++++++++++++++++- recommendations/RoofRecommendations.py | 76 +++++++++++++++++++++++--- 3 files changed, 143 insertions(+), 11 deletions(-) diff --git a/backend/app/plan/schemas.py b/backend/app/plan/schemas.py index edac31dc..7c352eba 100644 --- a/backend/app/plan/schemas.py +++ b/backend/app/plan/schemas.py @@ -9,7 +9,9 @@ TYPICAL_MEASURE_TYPES = [ ] WALL_INSULATION_MEASURES = ["internal_wall_insulation", "external_wall_insulation", "cavity_wall_insulation"] -ROOF_INSULATION_MEASURES = ["loft_insulation", "flat_roof_insulation", "room_roof_insulation"] +ROOF_INSULATION_MEASURES = [ + "loft_insulation", "flat_roof_insulation", "room_roof_insulation", "sloping_ceiling_insulation" +] # Both all and roof insulaiton measures are eligible for ECO4. These are the remaining fabric and heating measures # This is based on th measures we have recommendations for @@ -31,7 +33,7 @@ SPECIFIC_MEASURES = ( INSULATION_MEASURES = [ "internal_wall_insulation", "external_wall_insulation", "cavity_wall_insulation", - "loft_insulation", "flat_roof_insulation", "room_roof_insulation", + "loft_insulation", "flat_roof_insulation", "room_roof_insulation", "sloping_ceiling_insulation", "suspended_floor_insulation", "solid_floor_insulation", ] @@ -46,7 +48,9 @@ MEASURE_MAP = { "wall_insulation": [ "internal_wall_insulation", "external_wall_insulation", "cavity_wall_insulation", ], - "roof_insulation": ["loft_insulation", "flat_roof_insulation", "room_roof_insulation"], + "roof_insulation": [ + "loft_insulation", "flat_roof_insulation", "room_roof_insulation", "sloping_ceiling_insulation" + ], "floor_insulation": ["suspended_floor_insulation", "solid_floor_insulation"], "heating": ["boiler_upgrade", "high_heat_retention_storage_heaters", "air_source_heat_pump"], "windows": ["double_glazing", "secondary_glazing"], diff --git a/recommendations/Costs.py b/recommendations/Costs.py index 3a65312e..fd429afa 100644 --- a/recommendations/Costs.py +++ b/recommendations/Costs.py @@ -1,4 +1,6 @@ +from typing import Mapping, Any import numpy as np + from recommendations.county_to_region import county_to_region_map from utils.logger import setup_logger from backend.ml_models.AnnualBillSavings import AnnualBillSavings @@ -166,7 +168,8 @@ class Costs: "room_roof_insulation": 0.26, "heater_removal": 0.1, "sealing_open_fireplace": 0.1, - "mechanical_ventilation": 0.26 + "mechanical_ventilation": 0.26, + "sloping_ceiling_insulation": 0.26 # Similar to IWI so using the same contingency } # Preliminaries are a percentage of the total cost of the work and covers the cost of site-specific costs @@ -935,3 +938,66 @@ class Costs: "labour_hours": 80, "labour_days": 10, } + + @staticmethod + def _estimate_number_of_days_for_sloping_ceiling(insulation_roof_area: float) -> float: + """ + Estimate labour days required to insulate an existing sloping ceiling. + + Heuristic model based on retrofit guidance (Checkatrade, The Green Age) + and analogy with internal wall insulation. + + Assumptions: + - ~30 m² of sloping ceiling takes ~4 working days + - Small jobs still require multiple days (setup, stripping, reboarding) + - Larger areas benefit from economies of scale, but not linearly + + :param insulation_roof_area: m² of sloping ceiling to be insulated + """ + + base_days = 4 + base_area = 30 # m2 reference case + labour_exponent = 0.85 + min_days = 2 + + labour_days = max( + min_days, + base_days * (insulation_roof_area / base_area) ** labour_exponent + ) + + return labour_days + + @classmethod + def sloping_ceiling_insulation(cls, insulation_roof_area: float) -> Mapping[str, Any]: + """ + This costing for this is based on Checkatrade desktop research, since we are yet to receive installer quotes. + :param insulation_roof_area: Area of the sloping ceiling to be insulated + :return: + """ + ################ + # Assumptions + ################ + # Sources: + # https://www.checkatrade.com/blog/cost-guides/vaulted-ceiling-cost/ + # https://www.thegreenage.co.uk/can-i-insulate-my-sloping-ceiling/ + # These assumptions last updated 21/02/2026 + insulation_cost_per_m2 = 52 # The actual install process is quite similar to IWI + labour_rate = 250 # per day + contingency_rate = cls.CONTINGENCIES["sloping_ceiling_insulation"] + + labour_days = cls._estimate_number_of_days_for_sloping_ceiling(insulation_roof_area) + labour_hours = labour_days * 8 + + total = (insulation_cost_per_m2 * insulation_roof_area) + (labour_rate * labour_days) + + # Assume VAT included in the total => total is 120% of subtotal + vat = total - (total / 1.2) + + return { + "total": total, + "contingency": total * contingency_rate, + "contingency_rate": contingency_rate, + "vat": vat, + "labour_hours": labour_hours, + "labour_days": labour_days, + } diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 6625aeb0..baaaa547 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -324,10 +324,11 @@ class RoofRecommendations: ) self.estimated_u_value = u_value + # The Roof is already compliant - in this case, the u-value is beyond the requirements for + # Building Regs Part L and so we don't recommend anything if (u_value <= self.BUILDING_REGULATIONS_PART_L_MAX_U_VALUE) or all( m not in measures for m in MEASURE_MAP["roof_insulation"] ): - # The Roof is already compliant return non_invasive_recommendations = self.property.non_invasive_recommendations @@ -381,14 +382,12 @@ class RoofRecommendations: has_room_roof_recommendation=has_room_roof_recommendation ) - ################################################## + ################################################################ # ~~~~~ Loft Insulation Recommendation Logic ~~~~~ - ################################################## - # We firstly handle non-intrusive recommendations, which may override the normal roof insulation recommendations + ################################################################ if needs_loft_insulation: self.recommend_roof_insulation( u_value=u_value, - insulation_thickness=self.insulation_thickness, phase=phase, is_flat=False, is_pitched=True, @@ -396,10 +395,12 @@ class RoofRecommendations: ) return + ################################################################ + # ~~~~~ Flat Roof Insulation Recommendation Logic ~~~~~ + ################################################################ if needs_flat_roof_insulation: self.recommend_roof_insulation( u_value=u_value, - insulation_thickness=0, phase=phase, is_flat=True, is_pitched=False, @@ -407,12 +408,21 @@ class RoofRecommendations: ) return + ################################################################ + # ~~~~~ Room Roof Insulation Recommendation Logic ~~~~~ + ################################################################ # There are cases where the property might have a room roof as the second roof, but we have a recommendation for # it, so we allow this override if needs_rir_insulation: self.recommend_room_roof_insulation(u_value, phase, default_u_values) return + #################################################################################################### + # ~~~~~ Sloping Ceiling Insulation Recommendation Logic ~~~~~ + #################################################################################################### + if needs_sloping_ceiling: + self.recommend_sloping_ceiling() + raise NotImplementedError("Implement me") @staticmethod @@ -432,7 +442,7 @@ class RoofRecommendations: raise ValueError("Invalid material type") def recommend_roof_insulation( - self, u_value, insulation_thickness, phase, is_pitched, is_flat, default_u_values + self, u_value, phase, is_pitched, is_flat, default_u_values ): """ @@ -773,3 +783,55 @@ class RoofRecommendations: ) self.recommendations = recommendations + + def recommend_sloping_ceiling(self, phase: int, u_value, sloping_ceiling_recommendation: dict = None): + """ + Recommend insulation for a sloping ceiling + Since we don't have any materials from installers for this specific recommendation, we + do not iterate through any materials. Instead, we provide a single recommendation, we estimated + prices based on desk research. + :return: + """ + + new_description = "Pitched, insulated" + new_efficiency = "Good" + + roof_ending_config = RoofAttributes(new_description).process() + roof_simulation_config = check_simulation_difference( + new_config=roof_ending_config, old_config=self.property.roof, prefix="roof_" + ) + + # We pull out new u-values, based on 75mm of insulation, with u-values defined from Elmhurst + new_u_value = 0.5 # This doesn't change, regardless of starting u-value + + simulation_config = { + **roof_simulation_config, + "roof_thermal_transmittance_ending": new_u_value, + "roof_energy_eff_ending": new_efficiency + } + + cost_result = self.costs.sloping_ceiling_insulation( + roof_area=self.property.roof_area # For a pitched roof, this is the pitched roof area + ) + + self.recommendations = [ + { + "phase": phase, + "parts": [], + "type": "sloping_ceiling_insulation", + "measure_type": "sloping_ceiling_insulation", + "description": "Insulate sloping ceilings at the rafters and re-decorate", + "starting_u_value": u_value, + "new_u_value": None, + "sap_points": sloping_ceiling_recommendation.get("sap_points", None), + "simulation_config": simulation_config, + "description_simulation": { + "roof-description": new_description, + "roof-energy-eff": new_efficiency + }, + **cost_result, + "already_installed": "sloping_ceiling_insulation" in self.property.already_installed, + "survey": sloping_ceiling_recommendation.get("survey", None), + "innovation_rate": 0 + } + ] From ac749e427e2f8724077489cf6bff4a6adc6f2c32 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 26 Jan 2026 19:06:50 +0000 Subject: [PATCH 05/18] first implementation for sloping ceiling insulation --- .idea/copilot.data.migration.ask2agent.xml | 6 ++ recommendations/Costs.py | 16 ++-- recommendations/RoofRecommendations.py | 59 ++++++++------- .../tests/test_roof_recommendations.py | 74 +++++++++---------- 4 files changed, 84 insertions(+), 71 deletions(-) create mode 100644 .idea/copilot.data.migration.ask2agent.xml diff --git a/.idea/copilot.data.migration.ask2agent.xml b/.idea/copilot.data.migration.ask2agent.xml new file mode 100644 index 00000000..1f2ea11e --- /dev/null +++ b/.idea/copilot.data.migration.ask2agent.xml @@ -0,0 +1,6 @@ + + + + + \ No newline at end of file diff --git a/recommendations/Costs.py b/recommendations/Costs.py index fd429afa..5f312f63 100644 --- a/recommendations/Costs.py +++ b/recommendations/Costs.py @@ -947,6 +947,10 @@ class Costs: Heuristic model based on retrofit guidance (Checkatrade, The Green Age) and analogy with internal wall insulation. + See _estimate_number_of_days_for_solid_floor for detailed explanation regarding assumptions + and methodology, however for the purpose of placeholder, this function mimics the approach + to that method but is detached to allow for future changes + Assumptions: - ~30 m² of sloping ceiling takes ~4 working days - Small jobs still require multiple days (setup, stripping, reboarding) @@ -968,7 +972,7 @@ class Costs: return labour_days @classmethod - def sloping_ceiling_insulation(cls, insulation_roof_area: float) -> Mapping[str, Any]: + def sloping_ceiling_insulation(cls, insulation_roof_area: float) -> Mapping[str, float]: """ This costing for this is based on Checkatrade desktop research, since we are yet to receive installer quotes. :param insulation_roof_area: Area of the sloping ceiling to be insulated @@ -994,10 +998,10 @@ class Costs: vat = total - (total / 1.2) return { - "total": total, - "contingency": total * contingency_rate, + "total": float(total), + "contingency": float(total * contingency_rate), "contingency_rate": contingency_rate, - "vat": vat, - "labour_hours": labour_hours, - "labour_days": labour_days, + "vat": float(vat), + "labour_hours": float(labour_hours), + "labour_days": float(labour_days), } diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index baaaa547..06ff306b 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -2,7 +2,7 @@ import math import pandas as pd from backend.Property import Property from backend.app.plan.schemas import MEASURE_MAP -from typing import List +from typing import List, Mapping, Any from datatypes.enums import QuantityUnits from recommendations.recommendation_utils import ( get_roof_u_value, r_value_per_mm_to_u_value, calculate_u_value_uplift, is_diminishing_returns, @@ -119,24 +119,6 @@ class RoofRecommendations: return (full_insulated_room_roof or room_roof_insulated_at_rafters) and not has_non_invasive_recommendation - def recommend_sloping_ceiling(self): - """ - Sloping ceiling insulation recommendations are different from other roof types, though - the description of the roof appears to be quite similar to a roof with a loft. In order to - deduce the roof type, we apply the following logic: - - 1) If the roof is descrbed as pitched, insulated, without a loft insulation thickness, it's - an insulated sloped ceiling - 2) If the roof insulation is assumed, it implies that the surveyor could not gain access to the - roof and therefore it's a loft - 3) If it's a pitched roof that is uninsulated and is NOT assumed, and there is not loft insulation - recommendation, this implies that the surveyor was able to gain access to the roof and there was no - loft insulation recommendation so it must be a sloping ceiling since loft insulation is a default - recommendation for an uninsualted loft - :return: - """ - pass - @staticmethod def is_sloping_ceiling_appropriate( is_pitched: bool, is_loft: bool, is_assumed: bool, has_sloping_ceiling_recommendation: bool, @@ -207,7 +189,7 @@ class RoofRecommendations: @staticmethod def is_flat_roof_insulation_appropriate( - is_flat: bool, measures: List, has_flat_roof_recommendation: bool + is_flat: bool, measures: List, has_flat_roof_recommendation: bool, primary_roof_is_sloped: bool ) -> bool: """ Determine if flat roof insulation is appropriate @@ -215,12 +197,17 @@ class RoofRecommendations: :param measures: List - list of measures :param has_flat_roof_recommendation: Boolean - indicates whether or not there is a flat roof non-invasive recommendation + :param primary_roof_is_sloped: Boolean - indicates if the primary roof is flat :return: Boolean + + When checking if has_flat_roof_recommendation and primary_roof_is_sloped, we need to check both + conditions. This is because within a default EPC recommendation, the EPC will pair these recommendations + together. Therefore, weneed to ensure the primary roof isn't sloped """ flat_roof_in_measures = "flat_roof_insulation" in measures - return (is_flat and flat_roof_in_measures) or has_flat_roof_recommendation + return (is_flat and flat_roof_in_measures) or (has_flat_roof_recommendation and not primary_roof_is_sloped) @staticmethod def is_room_roof_insulation_appropriate( @@ -272,6 +259,8 @@ class RoofRecommendations: ): return False + return True + @staticmethod def _is_primary_roof_sloped( is_pitched: bool, is_loft: bool, is_assumed: bool @@ -365,15 +354,15 @@ class RoofRecommendations: has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, primary_roof_is_sloped=primary_roof_is_sloped ) - needs_loft_insulation = self.is_loft_insulation_appropriate( measures=measures, is_pitched=is_pitched, is_at_rafters=is_at_rafters, rir_over_loft=rir_over_loft, has_loft_insulation_recommendation=has_loft_insulation_recommendation, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation ) - needs_flat_roof_insulation = self.is_flat_roof_insulation_appropriate( - is_flat=is_flat, measures=measures, has_flat_roof_recommendation=has_flat_roof_recommendation + is_flat=is_flat, measures=measures, + has_flat_roof_recommendation=has_flat_roof_recommendation, + primary_roof_is_sloped=primary_roof_is_sloped ) needs_rir_insulation = self.is_room_roof_insulation_appropriate( is_room_roof=is_room_roof, @@ -784,15 +773,31 @@ class RoofRecommendations: self.recommendations = recommendations - def recommend_sloping_ceiling(self, phase: int, u_value, sloping_ceiling_recommendation: dict = None): + def recommend_sloping_ceiling(self, phase: int, u_value, non_invasive_recommendations: List[Mapping[str, Any]]): """ - Recommend insulation for a sloping ceiling + Sloping ceiling insulation recommendations are different from other roof types, though + the description of the roof appears to be quite similar to a roof with a loft. In order to + deduce the roof type, we apply the following logic: + + 1) If the roof is descrbed as pitched, insulated, without a loft insulation thickness, it's + an insulated sloped ceiling + 2) If the roof insulation is assumed, it implies that the surveyor could not gain access to the + roof and therefore it's a loft + 3) If it's a pitched roof that is uninsulated and is NOT assumed, and there is not loft insulation + recommendation, this implies that the surveyor was able to gain access to the roof and there was no + loft insulation recommendation so it must be a sloping ceiling since loft insulation is a default + recommendation for an uninsualted loft + Since we don't have any materials from installers for this specific recommendation, we do not iterate through any materials. Instead, we provide a single recommendation, we estimated prices based on desk research. :return: """ + sloping_ceiling_recommendation = next( + (x for x in non_invasive_recommendations if ["type"] == "sloping_ceiling_insulation"), {} + ) + new_description = "Pitched, insulated" new_efficiency = "Good" @@ -811,7 +816,7 @@ class RoofRecommendations: } cost_result = self.costs.sloping_ceiling_insulation( - roof_area=self.property.roof_area # For a pitched roof, this is the pitched roof area + insulation_roof_area=self.property.roof_area # For a pitched roof, this is the pitched roof area ) self.recommendations = [ diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index b8cea10b..ef02d050 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -1,8 +1,8 @@ +import pytest from backend.Property import Property +from etl.epc.Record import EPCRecord from recommendations.RoofRecommendations import RoofRecommendations from recommendations.tests.test_data.materials import materials -from etl.epc.Record import EPCRecord -import pytest class TestRoofRecommendations: @@ -405,40 +405,38 @@ class TestRoofRecommendations: assert not roof_recommender14.recommendations # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ - @pytest.mark.parameterize("roof", - [ - ( - # For this example, the roof is pitched, without insulation and the description - # isn't assumed - {'original_description': 'Pitched, no insulation', 'thermal_transmittance': None, - 'thermal_transmittance_unit': None, - 'is_pitched': True, 'is_roof_room': False, 'is_loft': False, 'is_flat': False, - 'is_thatched': False, - 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, - 'is_valid': True, - 'insulation_thickness': 'none'} - ) - ] - ) - def test_sloping_ceiling_valid(self, roof): - # All conditions are met and therefore we should produce a sloping ceiling recommendation + @pytest.mark.parametrize( + "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result", + [ + ( + { + 'original_description': 'Pitched, no insulation', + 'thermal_transmittance': None, + 'thermal_transmittance_unit': None, + 'is_pitched': True, + 'is_roof_room': False, + 'is_loft': False, + 'is_flat': False, + 'is_thatched': False, + 'is_at_rafters': False, + 'is_assumed': False, + 'has_dwelling_above': False, + 'is_valid': True, + 'insulation_thickness': 'none' + }, + True, + True, + True, + ) + ] + ) + def test_sloping_ceiling_valid( + self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result + ): assert RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=False, is_assumed=False, has_sloping_ceiling_recommendation=True - ) - - # One condition not met - we cannot verify - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=True, is_assumed=False, has_sloping_ceiling_recommendation=True - ) - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=False, is_loft=False, is_assumed=False, has_sloping_ceiling_recommendation=True, - primary_roof_is_sloped=True - ) - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=False, is_assumed=True, has_sloping_ceiling_recommendation=True, - primary_roof_is_sloped=True - ) - assert not RoofRecommendations.is_sloping_ceiling_appropriate( - is_pitched=True, is_loft=False, is_assumed=True, has_sloping_ceiling_recommendation=True, - primary_roof_is_sloped=True - ) + is_pitched=roof["is_pitched"], + is_loft=roof["is_loft"], + is_assumed=roof["is_assumed"], + has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, + primary_roof_is_sloped=primary_roof_is_sloped + ) == expected_result From b81b04a6b891a03b93ce6e6a510bbbd4e65c93f8 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 27 Jan 2026 08:43:58 +0000 Subject: [PATCH 06/18] added initial test --- recommendations/RoofRecommendations.py | 10 ++-- .../tests/test_roof_recommendations.py | 50 ++++++++++++++++++- .../tests/test_wall_recommendations.py | 2 - 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 06ff306b..d7eb5fbb 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -410,7 +410,12 @@ class RoofRecommendations: # ~~~~~ Sloping Ceiling Insulation Recommendation Logic ~~~~~ #################################################################################################### if needs_sloping_ceiling: - self.recommend_sloping_ceiling() + self.recommend_sloping_ceiling( + phase=phase, + u_value=u_value, + non_invasive_recommendations=non_invasive_recommendations + ) + return raise NotImplementedError("Implement me") @@ -453,7 +458,6 @@ class RoofRecommendations: could be traditional roofing materials like bitumen-based felt, rubber membranes like EPDM, or fiberglass. :param u_value: U-value of the roof before any retrofit measures have been installed - :param insulation_thickness: Existing Insulation thickness of the loft :param phase: Phase of the recommendation :param is_pitched: Is the roof pitched :param is_flat: Is the roof flat @@ -799,7 +803,7 @@ class RoofRecommendations: ) new_description = "Pitched, insulated" - new_efficiency = "Good" + new_efficiency = "Average" # 75mm insulation only results in average performance category roof_ending_config = RoofAttributes(new_description).process() roof_simulation_config = check_simulation_difference( diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index ef02d050..aa11fd28 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -1,4 +1,5 @@ import pytest +from unittest.mock import Mock from backend.Property import Property from etl.epc.Record import EPCRecord from recommendations.RoofRecommendations import RoofRecommendations @@ -430,7 +431,7 @@ class TestRoofRecommendations: ) ] ) - def test_sloping_ceiling_valid( + def test_is_sloping_ceiling_appropriate( self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result ): assert RoofRecommendations.is_sloping_ceiling_appropriate( @@ -440,3 +441,50 @@ class TestRoofRecommendations: has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, primary_roof_is_sloped=primary_roof_is_sloped ) == expected_result + + def test_sloping_ceiling_pitched_no_insulation(self): + property_instance = Mock( + id=0, + roof={ + 'original_description': 'Pitched, no insulation', 'clean_description': 'Pitched, no insulation', + 'thermal_transmittance': None, 'thermal_transmittance_unit': None, 'is_pitched': True, + 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, + 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': True, + 'insulation_thickness': 'none' + }, + roof_area=64.085, + data={"county": None, "local-authority-label": "Manchester"} + ) + property_instance.age_band = "D" + property_instance.already_installed = [] + property_instance.non_invasive_recommendations = [ + {'type': 'flat_roof_insulation', 'sap_points': 9, 'survey': True}, + {'type': 'sloping_ceiling_insulation', 'sap_points': 9, 'survey': True}, + {'type': 'cavity_wall_insulation', 'sap_points': 6, 'survey': True}, + {'type': 'suspended_floor_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'roomstat_programmer_trvs', 'sap_points': 3, 'survey': True}, + {'type': 'time_temperature_zone_control', 'sap_points': 3, 'survey': True}, + {'type': 'solar_pv', 'sap_points': 5, 'survey': True, 'suitable': True} + ] + + roof_recommender = RoofRecommendations(property_instance=property_instance, materials=[]) + assert not roof_recommender.recommendations + + roof_recommender.recommend(phase=0) + assert len(roof_recommender.recommendations) == 1 + + assert roof_recommender.recommendations[0]["type"] == "sloping_ceiling_insulation" + assert roof_recommender.recommendations[0]["measure_type"] == "sloping_ceiling_insulation" + assert ( + roof_recommender.recommendations[0]["description"] == + "Insulate sloping ceilings at the rafters and re-decorate" + ) + assert roof_recommender.recommendations[0]["simulation_config"] == { + 'roof_insulation_thickness_ending': 'average', + 'roof_thermal_transmittance_ending': 0.5, + 'roof_energy_eff_ending': 'Average' + } + + assert roof_recommender.recommendations[0]["description_simulation"] == { + 'roof-description': 'Pitched, insulated', 'roof-energy-eff': 'Good' + } diff --git a/recommendations/tests/test_wall_recommendations.py b/recommendations/tests/test_wall_recommendations.py index 18560118..c54582ad 100644 --- a/recommendations/tests/test_wall_recommendations.py +++ b/recommendations/tests/test_wall_recommendations.py @@ -1,6 +1,4 @@ -import os import pytest -import pickle import numpy as np from unittest.mock import Mock, MagicMock From 59c92574a3745196ffdd6a52fc39aa0c0fcdcbeb Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 27 Jan 2026 08:46:52 +0000 Subject: [PATCH 07/18] added costs unit test --- recommendations/tests/test_costs.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/recommendations/tests/test_costs.py b/recommendations/tests/test_costs.py index 752caf8c..10a63554 100644 --- a/recommendations/tests/test_costs.py +++ b/recommendations/tests/test_costs.py @@ -236,3 +236,11 @@ class TestCosts: ) assert result['total'] == pytest.approx(expected_cost, rel=0.01) + + def test_sloping_ceiling_insulation(self): + mock_property = Mock() + mock_property.data = {"county": "Mansfield"} + costs = Costs(mock_property) + res = costs.sloping_ceiling_insulation(insulation_roof_area=64.085) + assert res["total"] == 5238.713924924947 + assert res["contingency"] == 1362.0656204804861 From f8937c790aa98a3b63b071934699c5ef10f786c4 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 27 Jan 2026 09:29:56 +0000 Subject: [PATCH 08/18] added counter example --- recommendations/tests/test_roof_recommendations.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index aa11fd28..7bafdffa 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -428,6 +428,18 @@ class TestRoofRecommendations: True, True, True, + ), + ( + { + 'original_description': 'Pitched, insulated (assumed)', 'clean_description': 'Pitched, insulated', + 'thermal_transmittance': None, 'thermal_transmittance_unit': None, 'is_pitched': True, + 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, + 'is_at_rafters': False, 'is_assumed': True, 'has_dwelling_above': False, 'is_valid': True, + 'insulation_thickness': 'average' + }, + False, + False, + False ) ] ) From f123a7ab8998ac200d68d80247ddf97b70a3999f Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 27 Jan 2026 09:31:18 +0000 Subject: [PATCH 09/18] expected performance for sloping ceiling updated to good --- recommendations/tests/test_roof_recommendations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index 7bafdffa..e797892d 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -498,5 +498,5 @@ class TestRoofRecommendations: } assert roof_recommender.recommendations[0]["description_simulation"] == { - 'roof-description': 'Pitched, insulated', 'roof-energy-eff': 'Good' + 'roof-description': 'Pitched, insulated', 'roof-energy-eff': 'Average' } From 79ef0805c3d8e12082ff06b295949ac1ec707c6a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 27 Jan 2026 19:04:37 +0000 Subject: [PATCH 10/18] handling ambiguous cases for sloping ceiling vs loft insulation --- backend/Property.py | 2 + backend/engine/engine.py | 5 +- etl/find_my_epc/RetrieveFindMyEpc.py | 59 ++++- recommendations/RoofRecommendations.py | 217 ++++++++++++++-- .../tests/test_roof_recommendations.py | 241 ++++++++++++++++-- 5 files changed, 477 insertions(+), 47 deletions(-) diff --git a/backend/Property.py b/backend/Property.py index fa607cfd..c320fdd8 100644 --- a/backend/Property.py +++ b/backend/Property.py @@ -84,6 +84,7 @@ class Property: uprn=None, # Pass as an optional input property_valuation=None, already_installed=None, + find_my_epc_components=None, non_invasive_recommendations=None, measures=None, energy_assessment=None, @@ -114,6 +115,7 @@ class Property: non_invasive_recommendations['recommendations'] if non_invasive_recommendations else [] ) + self.find_my_epc_components = find_my_epc_components # Store the find my epc components # This is a list of measures that have been recommended for the property if isinstance(measures, list): self.measures = measures diff --git a/backend/engine/engine.py b/backend/engine/engine.py index a9156078..67353b7a 100644 --- a/backend/engine/engine.py +++ b/backend/engine/engine.py @@ -796,9 +796,9 @@ async def model_engine(body: PlanTriggerRequest): property_non_invasive_recommendations, patch = req_data.non_invasive_recommendations, req_data.patch # if we have a remote assment data type, we pull the additional data and include it - epc_page_source = {} + epc_page_source, find_my_epc_components = {}, [] if (body.event_type == "remote_assessment") and not (epc_searcher.newest_epc.get("estimated")): - property_non_invasive_recommendations, patch, epc_page_source = ( + property_non_invasive_recommendations, patch, epc_page_source, find_my_epc_components = ( RetrieveFindMyEpc.get_from_epc_with_fallback( epc=epc_searcher.newest_epc, epc_page=epc_page, @@ -834,6 +834,7 @@ async def model_engine(body: PlanTriggerRequest): postcode=epc_searcher.postcode_clean, epc_record=prepared_epc, already_installed=property_already_installed + eco_packages.get(property_id)[3], + find_my_epc_components=find_my_epc_components, property_valuation=req_data.valuation, non_invasive_recommendations=property_non_invasive_recommendations, energy_assessment=energy_assessment, diff --git a/etl/find_my_epc/RetrieveFindMyEpc.py b/etl/find_my_epc/RetrieveFindMyEpc.py index 82215443..8bdc45c5 100644 --- a/etl/find_my_epc/RetrieveFindMyEpc.py +++ b/etl/find_my_epc/RetrieveFindMyEpc.py @@ -36,6 +36,9 @@ class RetrieveFindMyEpc: self.rrn = rrn self.address_cleaned = self.address.replace(",", "").replace(" ", "").lower() + + # Containers for the extracted components + self.property_components = [] self.walls = [] self.address_postal_town = address_postal_town @@ -256,12 +259,10 @@ class RetrieveFindMyEpc: property_features_table = soup.find("tbody", class_="govuk-table__body") property_features_table = property_features_table.find_all("tr") - # Extract wall types - self.walls = [] - for row in property_features_table: - cells = row.find_all("td") - if row.find("th").text.strip() == "Wall": - self.walls.append(cells[0].text.strip()) + self.extract_property_components(property_features_table) + + # Extract walls + self.walls = [x["description"] for x in self.property_components if x["component_name"] == "Wall"] # Finally, we format the recommendations recommendations = self.format_recommendations(recommendations, assessment_data, sap_2012_date) @@ -424,6 +425,37 @@ class RetrieveFindMyEpc: return chosen_epc, epc_certificate + @staticmethod + def extract_property_components(property_features_table: list): + """ + Function to pull out a table for property components, marking their appearance index + :param property_features_table: The table of property features, as extracted by BeautifulSoup + :return: List of property components with appearance index + """ + property_components = [] + for row in property_features_table: + cells = row.find_all("td") + component_name = row.find("th").text.strip() + property_components.append( + { + "component_name": component_name, + "description": cells[0].text.strip(), + "efficiency": cells[1].text.strip(), + } + ) + # Add an appearance index, which will indicate if the component appears multiple times, so this + # becomes a reference for the building part the component is associated to (main, extensions, etc) + # We want to inject this appearance index into the component dictionaries + component_count = {} + for component in property_components: + name = component['component_name'] + if name not in component_count: + component_count[name] = 0 + component['appearance_index'] = component_count[name] + component_count[name] += 1 + + return property_components + def retrieve_newest_find_my_epc_data( self, sap_2012_date=None, return_page=False, epc_page_source=None, rrn=None ): @@ -577,12 +609,10 @@ class RetrieveFindMyEpc: property_features_table = address_res.find("tbody", class_="govuk-table__body") property_features_table = property_features_table.find_all("tr") - # Extract wall types - self.walls = [] - for row in property_features_table: - cells = row.find_all("td") - if row.find("th").text.strip() == "Wall": - self.walls.append(cells[0].text.strip()) + property_components = self.extract_property_components(property_features_table) + + # Extract walls + self.walls = [x["description"] for x in self.property_components if x["component_name"] == "Wall"] # Finally, we format the recommendations recommendations = self.format_recommendations(recommendations, assessment_data, sap_2012_date) @@ -615,6 +645,7 @@ class RetrieveFindMyEpc: "heating_text": heating_text, "hot_water_text": hot_water_text, "recommendations": recommendations, + "property_components": property_components, "epc_data": epc_data, **assessment_data, **low_carbon_energy_sources, @@ -804,7 +835,9 @@ class RetrieveFindMyEpc: "page_source": find_epc_data.get("page_source") } - return non_invasive_recommendations, patch, page_source + property_components = find_epc_data.get("property_components", []) + + return non_invasive_recommendations, patch, page_source, property_components @classmethod def get_from_epc_with_fallback( diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index d7eb5fbb..bcaea687 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -11,6 +11,7 @@ from recommendations.recommendation_utils import ( ) from recommendations.Costs import Costs from etl.epc_clean.epc_attributes.RoofAttributes import RoofAttributes +from backend.app.plan.schemas import ROOF_INSULATION_MEASURES class RoofRecommendations: @@ -122,7 +123,7 @@ class RoofRecommendations: @staticmethod def is_sloping_ceiling_appropriate( is_pitched: bool, is_loft: bool, is_assumed: bool, has_sloping_ceiling_recommendation: bool, - primary_roof_is_sloped: bool + primary_roof_is_sloped: bool, insulation_thickness: str ) -> bool: """ @@ -133,6 +134,7 @@ class RoofRecommendations: recommendation :param primary_roof_is_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to an extension) + :param insulation_thickness: String - insulation thickness of the roof :return: """ # We need to check: @@ -141,11 +143,27 @@ class RoofRecommendations: # 3) Is the insulation status NOT assumed # 4) Is there a sloping ceiling recommendation (this may relate to the primary or secondary roof) - # If we have a loft primary roof and sloping cei + # If we have a loft primary roof and sloping ceiling + + has_suitable_features = ( + is_pitched and not is_loft and not is_assumed and primary_roof_is_sloped + ) + + # Check if it needs a recommendation + needs_recommendation_condition1 = has_sloping_ceiling_recommendation | ( + insulation_thickness in ["below average"] + ) + + needs_recommendation_condition2 = has_sloping_ceiling_recommendation & ( + insulation_thickness in ["none"] + ) + + # If the insulation thickness is 'none' this isn't alone conclusive for us to determine if it's + # a sloped ceiling + needs_recommendation = needs_recommendation_condition1 | needs_recommendation_condition2 # The property is pitched, not a loft, not assumed and has a sloping ceiling rec - if (is_pitched and not is_loft and not is_assumed and has_sloping_ceiling_recommendation and - primary_roof_is_sloped): + if has_suitable_features and needs_recommendation: return True return False @@ -157,17 +175,18 @@ class RoofRecommendations: is_at_rafters: bool, rir_over_loft: bool, is_assumed: bool, + insulation_thickness: str, has_loft_insulation_recommendation: bool, has_sloping_ceiling_recommendation: bool ) -> bool: """ Determine if loft insulation is appropriate - :param non_invasive_recommendations: List - list of non-invasive recommendations :param measures: List - list of measures :param is_pitched: Boolean - indicates whether or not the roof is pitched :param is_at_rafters: Boolean - indicates whether or not the loft insulation is at rafters :param rir_over_loft: Boolean - indicates whether or not there we should be doing RIR insulation :param is_assumed: Boolean - indicates whether or not the roof insulation status is assumed + :param insulation_thickness: String - insulation thickness of the roof :param has_loft_insulation_recommendation: Boolean - indicates whether or not there is a loft insulation non-invasive recommendation :param has_sloping_ceiling_recommendation: Boolean - indicates whether or not there @@ -183,6 +202,15 @@ class RoofRecommendations: if is_pitched and not is_assumed and has_sloping_ceiling_recommendation: return False + # We check the insulation thickness. If it's one of the "average", "below average", "none" values, + + if ( + is_assumed and is_pitched and insulation_thickness in ["average", "below average", "above average"] + and not has_sloping_ceiling_recommendation and not has_loft_insulation_recommendation + ): + # This is a pitched roof, without access to the loft, with unknown insulation status + return True + return has_loft_insulation_recommendation or ( is_pitched and has_li_in_measures and not is_at_rafters ) and not rir_over_loft @@ -283,6 +311,146 @@ class RoofRecommendations: return False + @staticmethod + def _deduce_primary_roof(component_needs: dict) -> str: + """ + Helper function for deducing the primary roof type used by _handle_multi_roof_types + """ + + # Can a non-primary part satisfy loft insulation? + primary_needs_loft = component_needs[1]["needs_loft_insulation"] + secondary_needs_loft = any( + p['needs_loft_insulation'] for idx, p in component_needs.items() if idx != 1 + ) + + if primary_needs_loft and not secondary_needs_loft: + # Only option is loft + return "loft" + + primary_needs_sloping = component_needs[1]["needs_sloping_ceiling"] + secondary_needs_sloping = any( + p['needs_sloping_ceiling'] for idx, p in component_needs.items() if idx != 1 + ) + + if primary_needs_sloping and not secondary_needs_sloping: + # Only option is sloping ceiling + return "sloping_ceiling" + + return "loft_insulation" # Defer to the cheaper option + + def _handle_multi_roof_types( + self, + measures: List, + find_my_epc_components: List[Mapping[str, Any]], + non_invasive_recommendations: List[Mapping[str, Any]], + has_sloping_ceiling_recommendation: bool, + has_loft_insulation_recommendation: bool, + rir_over_loft: bool + ) -> tuple[bool, bool]: + """ + This is a rough function to handle some edge cases, where we have two roof descriptions where + both look like they could be sloping ceilings or lofts. In this case, we need to deduce + which roof is the primary roof, and therefore whether or not we should recommend sloping ceiling insulation + :param measures: List - list of measures + :param find_my_epc_components: List - list of components from find my epc + :param non_invasive_recommendations: List - list of non-invasive recommendations + :param has_sloping_ceiling_recommendation: Boolean - indicates whether or not there is a sloping ceiling + recommendation + :param has_loft_insulation_recommendation: Boolean - indicates whether or not there is a loft insulation + recommendation + :param rir_over_loft: Boolean - indicates whether or not there we should be doing RIR insulation + :return: tuple[bool, bool] - (needs_sloping_ceiling, needs_loft_insulation) + """ + + # We utilise the find my EPC data to solve cases where the primary roof and secondary roof + # being loft and sloped ceiling is ambiguous + # We need to: + # 1) Check if we have two roof types + # 2) check if both could be considered sloped + # 3) Check if we have two non-invasive recommendations for both roof types + # 4) Determine which roof is the primary roof + + # We check a specific condition - which will imply loft insulation isn't appropriate but room in roof + # insulation is + # 1) We have an uninsulated loft (assumed) + # 2) We have a non-intrusive recommendation for room in roof insulation + + # We only use this when we have sloping ceiling and loft insulation recommendations + # Components are indexed from 0 + roof_count = max( + x["appearance_index"] for x in find_my_epc_components if x["component_name"] == "Roof" + ) + 1 + + roof_non_invasive_recommendations = [ + x["type"] for x in non_invasive_recommendations if x['type'] in ROOF_INSULATION_MEASURES + ] + + has_both_recommendations = ( + "loft_insulation" in roof_non_invasive_recommendations and \ + "sloping_ceiling_insulation" in roof_non_invasive_recommendations + ) + + if (roof_count <= 1) or not has_both_recommendations: + if roof_count > 1: + if "loft_insulation" in roof_non_invasive_recommendations: + return False, True + + if "sloping_ceiling_insulation" in roof_non_invasive_recommendations: + return True, False + + return True, False # Indicates that the property needs sloping ceiling as we only run this in that case + + extracted_roof_descriptions = { + idx: { + "description": component["description"], + **RoofAttributes(component["description"]).process() + } for idx, component in enumerate(find_my_epc_components) if component["component_name"] == "Roof" + } + + component_needs = {} + for component_idx, mapped in extracted_roof_descriptions.items(): + is_pitched = mapped["is_pitched"] + is_loft = mapped["is_loft"] + is_assumed = mapped["is_assumed"] + insulation_thickness = mapped["insulation_thickness"] + is_at_rafters = mapped["is_at_rafters"] + + needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( + is_pitched=is_pitched, + is_loft=is_loft, + is_assumed=is_assumed, + has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, + primary_roof_is_sloped=True, + insulation_thickness=insulation_thickness + ) + # If the roof has some form of insulation already but isn't a loft, it's + # not a loft. E.g. "pitched, limited insulation" is for sloping ceiling, not loft + needs_loft_insulation = self.is_loft_insulation_appropriate( + measures=measures, + is_pitched=is_pitched, + is_at_rafters=is_at_rafters, + rir_over_loft=rir_over_loft, + insulation_thickness=insulation_thickness, + has_loft_insulation_recommendation=has_loft_insulation_recommendation, + is_assumed=is_assumed, + has_sloping_ceiling_recommendation=False + ) + + component_needs[component_idx] = { + "needs_sloping_ceiling": needs_sloping_ceiling, + "needs_loft_insulation": needs_loft_insulation + } + + # Given the results we determine if the primary roof is sloped. The situation we may be in is + # one where the only otion is to assign one of the primary or secondary roof as a loft or sloped ceiling + # forcing our hand on whether the primary roof is sloped + primary_roof_type = self._deduce_primary_roof(component_needs) + + if primary_roof_type in ["ambiguous", "sloping_ceiling"]: + return True, False # Set sloping ceiling to true, loft to false + + return False, True # Set sloping ceiling to false, loft to true + def recommend(self, phase: int, measures: List | None = None, default_u_values: bool = False): """ Main method to recommend roof insulation measures @@ -322,17 +490,13 @@ class RoofRecommendations: non_invasive_recommendations = self.property.non_invasive_recommendations - # We check a specific condition - which will imply loft insulation isn't appropriate but room in roof - # insulation is - # 1) We have an uninsulated loft (assumed) - # 2) We have a non-intrusive recommendation for room in roof insulation - is_pitched = self.property.roof["is_pitched"] is_loft = self.property.roof["is_loft"] is_assumed = self.property.roof["is_assumed"] is_at_rafters = self.property.roof["is_at_rafters"] is_flat = self.property.roof["is_flat"] is_room_roof = self.property.roof["is_roof_room"] + insulation_thickness = self.property.roof["insulation_thickness"] has_sloping_ceiling_recommendation = any( x["type"] == "sloping_ceiling_insulation" for x in non_invasive_recommendations @@ -343,24 +507,32 @@ class RoofRecommendations: primary_roof_is_sloped = self._is_primary_roof_sloped( is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed ) - rir_over_loft = ( is_pitched and self.property.roof["insulation_thickness"] == "none" and "room_in_roof_insulation" in [x["type"] for x in non_invasive_recommendations] ) needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( - is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed, + is_pitched=is_pitched, + is_loft=is_loft, + is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped + primary_roof_is_sloped=primary_roof_is_sloped, + insulation_thickness=insulation_thickness ) needs_loft_insulation = self.is_loft_insulation_appropriate( - measures=measures, is_pitched=is_pitched, is_at_rafters=is_at_rafters, - rir_over_loft=rir_over_loft, has_loft_insulation_recommendation=has_loft_insulation_recommendation, - is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation + measures=measures, + is_pitched=is_pitched, + is_at_rafters=is_at_rafters, + rir_over_loft=rir_over_loft, + insulation_thickness=insulation_thickness, + has_loft_insulation_recommendation=has_loft_insulation_recommendation, + is_assumed=is_assumed, + has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation ) needs_flat_roof_insulation = self.is_flat_roof_insulation_appropriate( - is_flat=is_flat, measures=measures, + is_flat=is_flat, + measures=measures, has_flat_roof_recommendation=has_flat_roof_recommendation, primary_roof_is_sloped=primary_roof_is_sloped ) @@ -371,6 +543,17 @@ class RoofRecommendations: has_room_roof_recommendation=has_room_roof_recommendation ) + # We handle possible multi roof types + if needs_sloping_ceiling: + needs_sloping_ceiling, needs_loft_insulation = self._handle_multi_roof_types( + measures=measures, + find_my_epc_components=self.property.find_my_epc_components, + non_invasive_recommendations=non_invasive_recommendations, + has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, + has_loft_insulation_recommendation=has_loft_insulation_recommendation, + rir_over_loft=rir_over_loft + ) + ################################################################ # ~~~~~ Loft Insulation Recommendation Logic ~~~~~ ################################################################ diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index e797892d..48e96af7 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -1,6 +1,7 @@ import pytest from unittest.mock import Mock from backend.Property import Property +from etl.customers.immo.pilot.asset_list import non_invasive_recommendations from etl.epc.Record import EPCRecord from recommendations.RoofRecommendations import RoofRecommendations from recommendations.tests.test_data.materials import materials @@ -407,7 +408,7 @@ class TestRoofRecommendations: # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ @pytest.mark.parametrize( - "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result", + "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, insulation_thickness, expected_result", [ ( { @@ -427,6 +428,7 @@ class TestRoofRecommendations: }, True, True, + "none", True, ), ( @@ -439,19 +441,22 @@ class TestRoofRecommendations: }, False, False, + "average", False ) ] ) def test_is_sloping_ceiling_appropriate( - self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, expected_result + self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, + insulation_thickness, expected_result ): assert RoofRecommendations.is_sloping_ceiling_appropriate( is_pitched=roof["is_pitched"], is_loft=roof["is_loft"], is_assumed=roof["is_assumed"], has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped + primary_roof_is_sloped=primary_roof_is_sloped, + insulation_thickness=insulation_thickness ) == expected_result def test_sloping_ceiling_pitched_no_insulation(self): @@ -465,19 +470,42 @@ class TestRoofRecommendations: 'insulation_thickness': 'none' }, roof_area=64.085, - data={"county": None, "local-authority-label": "Manchester"} + data={"county": None, "local-authority-label": "Manchester"}, + age_band="D", + already_installed=[], + non_invasive_recommendations=[ + {'type': 'flat_roof_insulation', 'sap_points': 9, 'survey': True}, + {'type': 'sloping_ceiling_insulation', 'sap_points': 9, 'survey': True}, + {'type': 'cavity_wall_insulation', 'sap_points': 6, 'survey': True}, + {'type': 'suspended_floor_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'roomstat_programmer_trvs', 'sap_points': 3, 'survey': True}, + {'type': 'time_temperature_zone_control', 'sap_points': 3, 'survey': True}, + {'type': 'solar_pv', 'sap_points': 5, 'survey': True, 'suitable': True} + ], + find_my_epc_components=[ + {'component_name': 'Wall', 'description': 'Solid brick, as built, no insulation (assumed)', + 'efficiency': 'Very poor', 'appearance_index': 0}, + {'component_name': 'Roof', 'description': 'Pitched, no insulation', 'efficiency': 'Very poor', + 'appearance_index': 0}, + {'component_name': 'Roof', 'description': 'Pitched, limited insulation', 'efficiency': 'Very poor', + 'appearance_index': 1}, + {'component_name': 'Window', 'description': 'Some multiple glazing', 'efficiency': 'Very poor', + 'appearance_index': 0}, + {'component_name': 'Main heating', 'description': 'Boiler and radiators, mains gas', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Main heating control', 'description': 'Programmer, room thermostat and TRVs', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Hot water', 'description': 'From main system', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Lighting', 'description': 'Low energy lighting in 28% of fixed outlets', + 'efficiency': 'Average', 'appearance_index': 0}, + {'component_name': 'Floor', 'description': 'Solid, no insulation (assumed)', 'efficiency': 'N/A', + 'appearance_index': 0}, + {'component_name': 'Secondary heating', 'description': 'None', 'efficiency': 'N/A', + 'appearance_index': 0} + ] + ) - property_instance.age_band = "D" - property_instance.already_installed = [] - property_instance.non_invasive_recommendations = [ - {'type': 'flat_roof_insulation', 'sap_points': 9, 'survey': True}, - {'type': 'sloping_ceiling_insulation', 'sap_points': 9, 'survey': True}, - {'type': 'cavity_wall_insulation', 'sap_points': 6, 'survey': True}, - {'type': 'suspended_floor_insulation', 'sap_points': 2, 'survey': True}, - {'type': 'roomstat_programmer_trvs', 'sap_points': 3, 'survey': True}, - {'type': 'time_temperature_zone_control', 'sap_points': 3, 'survey': True}, - {'type': 'solar_pv', 'sap_points': 5, 'survey': True, 'suitable': True} - ] roof_recommender = RoofRecommendations(property_instance=property_instance, materials=[]) assert not roof_recommender.recommendations @@ -500,3 +528,186 @@ class TestRoofRecommendations: assert roof_recommender.recommendations[0]["description_simulation"] == { 'roof-description': 'Pitched, insulated', 'roof-energy-eff': 'Average' } + + def test_ambiguous_sloping_ceiling_or_loft(self): + # In this case, we actually expect loft insulation to be recommended + property_instance = Mock( + id=0, + roof={ + # Roof looks like it could be a sloping ceiling but it's actually a loft + 'original_description': 'Pitched, no insulation', 'clean_description': 'Pitched, no insulation', + 'thermal_transmittance': None, 'thermal_transmittance_unit': None, 'is_pitched': True, + 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, + 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': True, + 'insulation_thickness': 'none' + }, + roof_area=197.748, + data={"county": None, "local-authority-label": "Manchester"}, + already_installed=[], + find_my_epc_components=[ + {'component_name': 'Wall', 'description': 'Solid brick, as built, no insulation (assumed)', + 'efficiency': 'Very poor', 'appearance_index': 0}, + {'component_name': 'Roof', 'description': 'Pitched, no insulation', 'efficiency': 'Very poor', + 'appearance_index': 0}, + {'component_name': 'Roof', 'description': 'Pitched, limited insulation', 'efficiency': 'Very poor', + 'appearance_index': 1}, + {'component_name': 'Window', 'description': 'Some multiple glazing', 'efficiency': 'Very poor', + 'appearance_index': 0}, + {'component_name': 'Main heating', 'description': 'Boiler and radiators, mains gas', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Main heating control', 'description': 'Programmer, room thermostat and TRVs', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Hot water', 'description': 'From main system', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Lighting', 'description': 'Low energy lighting in 28% of fixed outlets', + 'efficiency': 'Average', 'appearance_index': 0}, + {'component_name': 'Floor', 'description': 'Solid, no insulation (assumed)', 'efficiency': 'N/A', + 'appearance_index': 0}, + {'component_name': 'Secondary heating', 'description': 'None', 'efficiency': 'N/A', + 'appearance_index': 0} + ], + age_band="B", + non_invasive_recommendations=[ + {'type': 'loft_insulation', 'sap_points': 3, 'survey': True}, + {'type': 'flat_roof_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'sloping_ceiling_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'internal_wall_insulation', 'sap_points': 9, 'survey': True}, + {'type': 'draught_proofing', 'sap_points': 1, 'survey': True}, + {'type': 'low_energy_lighting', 'sap_points': 1, 'survey': True}, + {'type': 'solar_water_heating', 'sap_points': 1, 'survey': True}, + {'type': 'double_glazing', 'sap_points': 3, 'survey': True}, + {'type': 'solar_pv', 'sap_points': 4, 'survey': True, 'suitable': True} + ], + insulation_floor_area=162 + ) + + roof_recommender = RoofRecommendations(property_instance=property_instance, materials=materials) + assert not roof_recommender.recommendations + + roof_recommender.recommend(phase=0) + assert len(roof_recommender.recommendations) == 3 + + # Should all be loft insulation recommendations + assert all( + rec["type"] == "loft_insulation" for rec in roof_recommender.recommendations + ) + + def test_no_access_pitched_roof_assumed(self): + """ + In this case, the roof will have been surveyed as pitched, but the surveyor won't + have gotten access to the property to check the insulation. Therefore, we + recommend loft insulation. We assume that the roof is a locked off loft + :return: + """ + + property_instance = Mock( + id=0, + roof={ + 'original_description': 'Pitched, limited insulation (assumed)', + 'clean_description': 'Pitched, limited insulation', 'thermal_transmittance': None, + 'thermal_transmittance_unit': None, 'is_pitched': True, 'is_roof_room': False, 'is_loft': False, + 'is_flat': False, 'is_thatched': False, 'is_at_rafters': False, 'is_assumed': True, + 'has_dwelling_above': False, 'is_valid': True, 'insulation_thickness': 'below average' + }, + roof_area=73.24, + data={"county": None, "local-authority-label": "Manchester"}, + already_installed=[], + find_my_epc_components=[ + {'component_name': 'Wall', 'description': 'Solid brick, as built, no insulation (assumed)', + 'efficiency': 'Very poor', 'appearance_index': 0}, + {'component_name': 'Wall', 'description': 'System built, as built, no insulation (assumed)', + 'efficiency': 'Poor', 'appearance_index': 1}, + {'component_name': 'Wall', 'description': 'Cavity wall, filled cavity', 'efficiency': 'Average', + 'appearance_index': 2}, + {'component_name': 'Roof', 'description': 'Pitched, limited insulation (assumed)', + 'efficiency': 'Very poor', 'appearance_index': 0}, + {'component_name': 'Window', 'description': 'Fully double glazed', 'efficiency': 'Average', + 'appearance_index': 0}, + {'component_name': 'Main heating', 'description': 'Boiler and radiators, mains gas', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Main heating control', 'description': 'Programmer and room thermostat', + 'efficiency': 'Average', 'appearance_index': 0}, + {'component_name': 'Hot water', 'description': 'From main system', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Lighting', 'description': 'Low energy lighting in 75% of fixed outlets', + 'efficiency': 'Very good', 'appearance_index': 0}, + {'component_name': 'Roof', 'description': '(another dwelling above)', 'efficiency': 'N/A', + 'appearance_index': 1}, + {'component_name': 'Floor', 'description': 'Suspended, no insulation (assumed)', 'efficiency': 'N/A', + 'appearance_index': 0}, + {'component_name': 'Floor', 'description': 'Solid, no insulation (assumed)', 'efficiency': 'N/A', + 'appearance_index': 1}, + {'component_name': 'Secondary heating', 'description': 'None', 'efficiency': 'N/A', + 'appearance_index': 0} + ], + age_band="B", + non_invasive_recommendations=[ + {'type': 'internal_wall_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'suspended_floor_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'solid_floor_insulation', 'sap_points': 1, 'survey': True}, + {'type': 'low_energy_lighting', 'sap_points': 0, 'survey': True} + ], + insulation_floor_area=60 + ) + + roof_recommender = RoofRecommendations(property_instance=property_instance, materials=materials) + assert not roof_recommender.recommendations + + roof_recommender.recommend(phase=0) + assert len(roof_recommender.recommendations) == 3 + + # Should all be loft insulation recommendations + assert all( + rec["type"] == "loft_insulation" for rec in roof_recommender.recommendations + ) + + def test_traditional_loft_insulation(self): + property_instance = Mock( + id=0, + roof={ + 'original_description': 'Pitched, no insulation', 'clean_description': 'Pitched, no insulation', + 'thermal_transmittance': None, 'thermal_transmittance_unit': None, 'is_pitched': True, + 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, + 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': True, + 'insulation_thickness': 'none' + }, + roof_area=48.82666666666667, + data={"county": None, "local-authority-label": "Manchester"}, + already_installed=[], + find_my_epc_components=[ + {'component_name': 'Wall', 'description': 'Cavity wall, filled cavity', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Roof', 'description': 'Pitched, no insulation', 'efficiency': 'Very poor', + 'appearance_index': 0}, + {'component_name': 'Window', 'description': 'Fully double glazed', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Main heating', 'description': 'Boiler and radiators, mains gas', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Main heating control', 'description': 'TRVs and bypass', 'efficiency': 'Average', + 'appearance_index': 0}, + {'component_name': 'Hot water', 'description': 'From main system', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Lighting', 'description': 'Low energy lighting in all fixed outlets', + 'efficiency': 'Very good', 'appearance_index': 0}, + {'component_name': 'Floor', 'description': 'Solid, no insulation (assumed)', 'efficiency': 'N/A', + 'appearance_index': 0}, + {'component_name': 'Secondary heating', 'description': 'Room heaters, electric', 'efficiency': 'N/A', + 'appearance_index': 0} + ], + age_band="F", + non_invasive_recommendations=[ + {'type': 'loft_insulation', 'sap_points': 9, 'survey': True}, + {'type': 'solid_floor_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'solar_water_heating', 'sap_points': 1, 'survey': True}, + {'type': 'solar_pv', 'sap_points': 11, 'survey': True, 'suitable': True} + ], + insulation_floor_area=40.0 + ) + + roof_recommender = RoofRecommendations(property_instance=property_instance, materials=materials) + assert not roof_recommender.recommendations + + roof_recommender.recommend(0) + assert len(roof_recommender.recommendations) == 3 + # should all be loft insulation recommendations + assert all(rec["type"] == "loft_insulation" for rec in roof_recommender.recommendations) From 81fc264afec0f1b128262a8590e46da3e3988b4b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 09:32:32 +0000 Subject: [PATCH 11/18] handling ambiguous cases for sloping ceiling vs loft insulation --- recommendations/RoofRecommendations.py | 15 ++++- .../tests/test_roof_recommendations.py | 61 ++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index bcaea687..578173bb 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -122,12 +122,16 @@ class RoofRecommendations: @staticmethod def is_sloping_ceiling_appropriate( - is_pitched: bool, is_loft: bool, is_assumed: bool, has_sloping_ceiling_recommendation: bool, - primary_roof_is_sloped: bool, insulation_thickness: str + is_pitched: bool, is_loft: bool, is_assumed: bool, + is_flat: bool, + has_sloping_ceiling_recommendation: bool, + primary_roof_is_sloped: bool, insulation_thickness: str, + has_loft_insulation_recommendation: bool ) -> bool: """ :param is_pitched: Boolean - indicates whether or not the roof is pitched + :param is_flat: Boolean - indicates whether or not the roof is flat :param is_loft: Boolean - indicates whether or not the roof is described as a loft :param is_assumed: Boolean - indiates if the assessment of the roof is assumed or actually confirmed :param has_sloping_ceiling_recommendation: Boolean - indicates if the property has a sloping ceiling @@ -135,6 +139,7 @@ class RoofRecommendations: :param primary_roof_is_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to an extension) :param insulation_thickness: String - insulation thickness of the roof + :param has_loft_insulation_recommendation: Boolean - indicates whether or not there :return: """ # We need to check: @@ -166,6 +171,11 @@ class RoofRecommendations: if has_suitable_features and needs_recommendation: return True + # In this case, we have an assumed pitched roof with average or below average insulation + # but a sloping ceiling insulation without loft + if has_sloping_ceiling_recommendation and not has_loft_insulation_recommendation and not is_flat: + return True + return False @staticmethod @@ -504,6 +514,7 @@ class RoofRecommendations: has_loft_insulation_recommendation = any(x["type"] == "loft_insulation" for x in non_invasive_recommendations) has_flat_roof_recommendation = any(x["type"] == "flat_roof_insulation" for x in non_invasive_recommendations) has_room_roof_recommendation = any(x["type"] == "room_roof_insulation" for x in non_invasive_recommendations) + # Very naive condition primary_roof_is_sloped = self._is_primary_roof_sloped( is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed ) diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index 48e96af7..f022f9b7 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -1,7 +1,6 @@ import pytest from unittest.mock import Mock from backend.Property import Property -from etl.customers.immo.pilot.asset_list import non_invasive_recommendations from etl.epc.Record import EPCRecord from recommendations.RoofRecommendations import RoofRecommendations from recommendations.tests.test_data.materials import materials @@ -711,3 +710,63 @@ class TestRoofRecommendations: assert len(roof_recommender.recommendations) == 3 # should all be loft insulation recommendations assert all(rec["type"] == "loft_insulation" for rec in roof_recommender.recommendations) + + def sloping_ceiling_limited_insulation(self): + property_instance = Mock( + id=0, + roof={ + "original_description": 'Pitched, limited insulation (assumed)', + 'clean_description': 'Pitched, limited insulation', + 'thermal_transmittance': None, 'thermal_transmittance_unit': None, 'is_pitched': True, + 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, 'is_at_rafters': False, + 'is_assumed': True, 'has_dwelling_above': False, 'is_valid': True, + 'insulation_thickness': 'below average' + }, + roof_area=35, + data={"county": None, "local-authority-label": "Manchester"}, + already_installed=[], + find_my_epc_components=[ + {'component_name': 'Wall', 'description': 'Cavity wall, as built, no insulation (assumed)', + 'efficiency': 'poor', 'appearance_index': 0}, + {'component_name': 'Roof', 'description': 'Pitched, limited insulation (assumed)', + 'efficiency': 'Very poor', 'appearance_index': 0}, + {'component_name': 'Window', 'description': 'Fully double glazed', 'efficiency': 'Average', + 'appearance_index': 0}, + {'component_name': 'Main heating', 'description': 'Boiler and radiators, mains gas', + 'efficiency': 'Good', 'appearance_index': 0}, + {'component_name': 'Main heating control', 'description': 'TRVs and bypass', + 'efficiency': 'Average', 'appearance_index': 0}, + {'component_name': 'Hot water', 'description': 'From main system', 'efficiency': 'Good', + 'appearance_index': 0}, + {'component_name': 'Lighting', 'description': 'Low energy lighting in all fixed outlets', + 'efficiency': 'Very good', 'appearance_index': 0}, + {'component_name': 'Floor', 'description': '(another dwelling below)', 'efficiency': 'N/A', + 'appearance_index': 0}, + {'component_name': 'Secondary heating', 'description': 'None', 'efficiency': 'N/A', + 'appearance_index': 0} + ], + age_band="B", + non_invasive_recommendations=[ + {'type': 'sloping_ceiling_insulation', 'sap_points': 2, 'survey': True}, + {'type': 'flat_roof_insulation', 'sap_points': 2, 'survey': True}, + ], + ) + + # We expect a sloping ceiling insulation recommendation + roof_recommender = RoofRecommendations(property_instance=property_instance, materials=materials) + assert not roof_recommender.recommendations + + roof_recommender.recommend(phase=0) + assert len(roof_recommender.recommendations) == 1 + assert roof_recommender.recommendations[0]["type"] == "sloping_ceiling_insulation" + assert roof_recommender.recommendations[0]["measure_type"] == "sloping_ceiling_insulation" + assert roof_recommender.recommendations[0]["description"] == \ + "Insulate sloping ceilings at the rafters and re-decorate" + assert roof_recommender.recommendations[0]["simulation_config"] == { + 'roof_insulation_thickness_ending': 'average', + 'roof_thermal_transmittance_ending': 0.5, + 'roof_energy_eff_ending': 'Average' + } + assert roof_recommender.recommendations[0]["description_simulation"] == { + 'roof-description': 'Pitched, insulated', 'roof-energy-eff': 'Average' + } From 5f463efe7d73f87805ec4642afe4cb0d6f25538a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 09:36:16 +0000 Subject: [PATCH 12/18] fixed last sloping ceiling recommendation with limited insulation --- recommendations/RoofRecommendations.py | 9 +++++++-- recommendations/tests/test_roof_recommendations.py | 11 ++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 578173bb..08fceb32 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -424,14 +424,17 @@ class RoofRecommendations: is_assumed = mapped["is_assumed"] insulation_thickness = mapped["insulation_thickness"] is_at_rafters = mapped["is_at_rafters"] + is_flat = mapped["is_flat"] needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( + is_flat=is_flat, is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, primary_roof_is_sloped=True, - insulation_thickness=insulation_thickness + insulation_thickness=insulation_thickness, + has_loft_insulation_recommendation=has_loft_insulation_recommendation ) # If the roof has some form of insulation already but isn't a loft, it's # not a loft. E.g. "pitched, limited insulation" is for sloping ceiling, not loft @@ -525,11 +528,13 @@ class RoofRecommendations: needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( is_pitched=is_pitched, + is_flat=is_flat, is_loft=is_loft, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, primary_roof_is_sloped=primary_roof_is_sloped, - insulation_thickness=insulation_thickness + insulation_thickness=insulation_thickness, + has_loft_insulation_recommendation=has_loft_insulation_recommendation ) needs_loft_insulation = self.is_loft_insulation_appropriate( measures=measures, diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index f022f9b7..9f39779c 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -407,7 +407,8 @@ class TestRoofRecommendations: # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ @pytest.mark.parametrize( - "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, insulation_thickness, expected_result", + "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, insulation_thickness, " + "has_loft_insulation_recommendation, expected_result", [ ( { @@ -428,6 +429,7 @@ class TestRoofRecommendations: True, True, "none", + False, True, ), ( @@ -441,21 +443,24 @@ class TestRoofRecommendations: False, False, "average", + False, False ) ] ) def test_is_sloping_ceiling_appropriate( self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, - insulation_thickness, expected_result + insulation_thickness, has_loft_insulation_recommendation, expected_result ): assert RoofRecommendations.is_sloping_ceiling_appropriate( + is_flat=roof["is_flat"], is_pitched=roof["is_pitched"], is_loft=roof["is_loft"], is_assumed=roof["is_assumed"], has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, primary_roof_is_sloped=primary_roof_is_sloped, - insulation_thickness=insulation_thickness + insulation_thickness=insulation_thickness, + has_loft_insulation_recommendation=has_loft_insulation_recommendation ) == expected_result def test_sloping_ceiling_pitched_no_insulation(self): From c1782e6e310282e3c7c8a719f684901bb75465f1 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 10:00:20 +0000 Subject: [PATCH 13/18] add specific return names in handling multi roof types --- recommendations/RoofRecommendations.py | 78 +++++++++++++------ .../tests/test_roof_recommendations.py | 6 +- 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 08fceb32..5eb81439 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -122,10 +122,13 @@ class RoofRecommendations: @staticmethod def is_sloping_ceiling_appropriate( - is_pitched: bool, is_loft: bool, is_assumed: bool, + is_pitched: bool, + is_loft: bool, + is_assumed: bool, is_flat: bool, has_sloping_ceiling_recommendation: bool, - primary_roof_is_sloped: bool, insulation_thickness: str, + primary_roof_looks_sloped: bool, + insulation_thickness: str, has_loft_insulation_recommendation: bool ) -> bool: """ @@ -136,7 +139,7 @@ class RoofRecommendations: :param is_assumed: Boolean - indiates if the assessment of the roof is assumed or actually confirmed :param has_sloping_ceiling_recommendation: Boolean - indicates if the property has a sloping ceiling recommendation - :param primary_roof_is_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to + :param primary_roof_looks_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to an extension) :param insulation_thickness: String - insulation thickness of the roof :param has_loft_insulation_recommendation: Boolean - indicates whether or not there @@ -151,7 +154,7 @@ class RoofRecommendations: # If we have a loft primary roof and sloping ceiling has_suitable_features = ( - is_pitched and not is_loft and not is_assumed and primary_roof_is_sloped + is_pitched and not is_loft and not is_assumed and primary_roof_looks_sloped ) # Check if it needs a recommendation @@ -227,7 +230,7 @@ class RoofRecommendations: @staticmethod def is_flat_roof_insulation_appropriate( - is_flat: bool, measures: List, has_flat_roof_recommendation: bool, primary_roof_is_sloped: bool + is_flat: bool, measures: List, has_flat_roof_recommendation: bool, primary_roof_looks_sloped: bool ) -> bool: """ Determine if flat roof insulation is appropriate @@ -235,17 +238,17 @@ class RoofRecommendations: :param measures: List - list of measures :param has_flat_roof_recommendation: Boolean - indicates whether or not there is a flat roof non-invasive recommendation - :param primary_roof_is_sloped: Boolean - indicates if the primary roof is flat + :param primary_roof_looks_sloped: Boolean - indicates if the primary roof looks like a sloped roof :return: Boolean - When checking if has_flat_roof_recommendation and primary_roof_is_sloped, we need to check both + When checking if has_flat_roof_recommendation and primary_roof_looks_sloped, we need to check both conditions. This is because within a default EPC recommendation, the EPC will pair these recommendations together. Therefore, weneed to ensure the primary roof isn't sloped """ flat_roof_in_measures = "flat_roof_insulation" in measures - return (is_flat and flat_roof_in_measures) or (has_flat_roof_recommendation and not primary_roof_is_sloped) + return (is_flat and flat_roof_in_measures) or (has_flat_roof_recommendation and not primary_roof_looks_sloped) @staticmethod def is_room_roof_insulation_appropriate( @@ -300,7 +303,7 @@ class RoofRecommendations: return True @staticmethod - def _is_primary_roof_sloped( + def _does_primary_roof_look_sloped( is_pitched: bool, is_loft: bool, is_assumed: bool ): """ @@ -387,6 +390,10 @@ class RoofRecommendations: # We only use this when we have sloping ceiling and loft insulation recommendations # Components are indexed from 0 + + needs_sloping = True + needs_loft = True + roof_count = max( x["appearance_index"] for x in find_my_epc_components if x["component_name"] == "Roof" ) + 1 @@ -403,12 +410,13 @@ class RoofRecommendations: if (roof_count <= 1) or not has_both_recommendations: if roof_count > 1: if "loft_insulation" in roof_non_invasive_recommendations: - return False, True + return not needs_sloping, needs_loft if "sloping_ceiling_insulation" in roof_non_invasive_recommendations: - return True, False + return needs_sloping, not needs_loft - return True, False # Indicates that the property needs sloping ceiling as we only run this in that case + return needs_sloping, not needs_loft # Indicates that the property needs sloping ceiling as we only run + # this in that case extracted_roof_descriptions = { idx: { @@ -432,7 +440,7 @@ class RoofRecommendations: is_loft=is_loft, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=True, + primary_roof_looks_sloped=True, insulation_thickness=insulation_thickness, has_loft_insulation_recommendation=has_loft_insulation_recommendation ) @@ -460,9 +468,9 @@ class RoofRecommendations: primary_roof_type = self._deduce_primary_roof(component_needs) if primary_roof_type in ["ambiguous", "sloping_ceiling"]: - return True, False # Set sloping ceiling to true, loft to false + return needs_sloping, not needs_loft # Set sloping ceiling to true, loft to false - return False, True # Set sloping ceiling to false, loft to true + return not needs_sloping, needs_loft # Set sloping ceiling to false, loft to true def recommend(self, phase: int, measures: List | None = None, default_u_values: bool = False): """ @@ -479,6 +487,10 @@ class RoofRecommendations: property_needs_roof_recommendation = self._does_roof_need_recommendation(measures, u_value) if not property_needs_roof_recommendation: + # Roof is either: + # - already sufficiently insulated + # - unsuitable (dwelling above, thatched, etc.) + # - not matching available measures return u_value = get_roof_u_value( @@ -517,13 +529,14 @@ class RoofRecommendations: has_loft_insulation_recommendation = any(x["type"] == "loft_insulation" for x in non_invasive_recommendations) has_flat_roof_recommendation = any(x["type"] == "flat_roof_insulation" for x in non_invasive_recommendations) has_room_roof_recommendation = any(x["type"] == "room_roof_insulation" for x in non_invasive_recommendations) - # Very naive condition - primary_roof_is_sloped = self._is_primary_roof_sloped( + + primary_roof_looks_sloped = self._does_primary_roof_look_sloped( is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed ) rir_over_loft = ( - is_pitched and self.property.roof["insulation_thickness"] == "none" and - "room_in_roof_insulation" in [x["type"] for x in non_invasive_recommendations] + is_pitched and + self.property.roof["insulation_thickness"] == "none" and + has_room_roof_recommendation ) needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( @@ -532,7 +545,7 @@ class RoofRecommendations: is_loft=is_loft, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped, + primary_roof_looks_sloped=primary_roof_looks_sloped, insulation_thickness=insulation_thickness, has_loft_insulation_recommendation=has_loft_insulation_recommendation ) @@ -550,7 +563,7 @@ class RoofRecommendations: is_flat=is_flat, measures=measures, has_flat_roof_recommendation=has_flat_roof_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped + primary_roof_looks_sloped=primary_roof_looks_sloped ) needs_rir_insulation = self.is_room_roof_insulation_appropriate( is_room_roof=is_room_roof, @@ -561,6 +574,9 @@ class RoofRecommendations: # We handle possible multi roof types if needs_sloping_ceiling: + # Multi-roof override: + # In ambiguous cases (extensions, mixed descriptions), EPC component analysis + # may force us to choose between loft vs sloping ceiling. needs_sloping_ceiling, needs_loft_insulation = self._handle_multi_roof_types( measures=measures, find_my_epc_components=self.property.find_my_epc_components, @@ -569,6 +585,17 @@ class RoofRecommendations: has_loft_insulation_recommendation=has_loft_insulation_recommendation, rir_over_loft=rir_over_loft ) + # Explicit override + needs_flat_roof_insulation = False + needs_rir_insulation = False + if needs_sloping_ceiling and needs_loft_insulation: + raise RuntimeError( + "Multi-roof resolution produced conflicting outcomes: " + "both sloping ceiling and loft insulation required" + ) + + # Retrofit precedence (least → most invasive): + # Loft > Flat roof > Room in roof > Sloping ceiling ################################################################ # ~~~~~ Loft Insulation Recommendation Logic ~~~~~ @@ -616,7 +643,14 @@ class RoofRecommendations: ) return - raise NotImplementedError("Implement me") + raise RuntimeError( + "Roof recommendation undecidable. " + f"needs_loft={needs_loft_insulation}, " + f"needs_flat={needs_flat_roof_insulation}, " + f"needs_rir={needs_rir_insulation}, " + f"needs_sloping={needs_sloping_ceiling}, " + f"roof={self.property.roof}" + ) @staticmethod def make_roof_insulation_description(material): diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index 9f39779c..0879757f 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -407,7 +407,7 @@ class TestRoofRecommendations: # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ @pytest.mark.parametrize( - "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, insulation_thickness, " + "roof, has_sloping_ceiling_recommendation, primary_roof_looks_sloped, insulation_thickness, " "has_loft_insulation_recommendation, expected_result", [ ( @@ -449,7 +449,7 @@ class TestRoofRecommendations: ] ) def test_is_sloping_ceiling_appropriate( - self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, + self, roof, has_sloping_ceiling_recommendation, primary_roof_looks_sloped, insulation_thickness, has_loft_insulation_recommendation, expected_result ): assert RoofRecommendations.is_sloping_ceiling_appropriate( @@ -458,7 +458,7 @@ class TestRoofRecommendations: is_loft=roof["is_loft"], is_assumed=roof["is_assumed"], has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped, + primary_roof_looks_sloped=primary_roof_looks_sloped, insulation_thickness=insulation_thickness, has_loft_insulation_recommendation=has_loft_insulation_recommendation ) == expected_result From fad9512fd70648b15b79aac08d7c6067eb3b9712 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 12:08:31 +0000 Subject: [PATCH 14/18] removed self.property_components from find my epc --- etl/find_my_epc/RetrieveFindMyEpc.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/etl/find_my_epc/RetrieveFindMyEpc.py b/etl/find_my_epc/RetrieveFindMyEpc.py index 8bdc45c5..392e6aaa 100644 --- a/etl/find_my_epc/RetrieveFindMyEpc.py +++ b/etl/find_my_epc/RetrieveFindMyEpc.py @@ -38,7 +38,6 @@ class RetrieveFindMyEpc: self.address_cleaned = self.address.replace(",", "").replace(" ", "").lower() # Containers for the extracted components - self.property_components = [] self.walls = [] self.address_postal_town = address_postal_town @@ -259,10 +258,10 @@ class RetrieveFindMyEpc: property_features_table = soup.find("tbody", class_="govuk-table__body") property_features_table = property_features_table.find_all("tr") - self.extract_property_components(property_features_table) + property_components = self.extract_property_components(property_features_table) # Extract walls - self.walls = [x["description"] for x in self.property_components if x["component_name"] == "Wall"] + self.walls = [x["description"] for x in property_components if x["component_name"] == "Wall"] # Finally, we format the recommendations recommendations = self.format_recommendations(recommendations, assessment_data, sap_2012_date) @@ -612,7 +611,7 @@ class RetrieveFindMyEpc: property_components = self.extract_property_components(property_features_table) # Extract walls - self.walls = [x["description"] for x in self.property_components if x["component_name"] == "Wall"] + self.walls = [x["description"] for x in property_components if x["component_name"] == "Wall"] # Finally, we format the recommendations recommendations = self.format_recommendations(recommendations, assessment_data, sap_2012_date) From 491ef6c2e89ba8030cf0213c576c85b9c9e64bac Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 12:10:18 +0000 Subject: [PATCH 15/18] added sloping ceiling insulation into create recommendation scoring data --- backend/Property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/Property.py b/backend/Property.py index c320fdd8..09c1d8ed 100644 --- a/backend/Property.py +++ b/backend/Property.py @@ -576,7 +576,7 @@ class Property: "solid_floor_insulation", "suspended_floor_insulation", "windows_glazing", "solar_pv", "heating", "hot_water_tank_insulation", "heating_control", "secondary_heating", "cylinder_thermostat", "mixed_glazing", - "extension_cavity_wall_insulation", "mechanical_ventilation", + "extension_cavity_wall_insulation", "mechanical_ventilation", "sloping_ceiling_insulation" ]: raise NotImplementedError( "Implement me, given type %s" % recommendation["type"] From 89262ff3dd09a887602e9c0821a670e751355ad9 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 12:35:25 +0000 Subject: [PATCH 16/18] added missing sloiping ceiling --- backend/Property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/Property.py b/backend/Property.py index 09c1d8ed..14f7e03f 100644 --- a/backend/Property.py +++ b/backend/Property.py @@ -553,7 +553,7 @@ class Property: "internal_wall_insulation", "external_wall_insulation", "cavity_wall_insulation", "cylinder_thermostat", "loft_insulation", "room_roof_insulation", "flat_roof_insulation", "solid_floor_insulation", "suspended_floor_insulation", "mixed_glazing", - "windows_glazing", "mechanical_ventilation", "solar_pv" + "windows_glazing", "mechanical_ventilation", "solar_pv", "sloping_ceiling_insulation" ]: # We update the data, as defined in the recommendaton for prefix in ["walls", "roof", "floor"]: From 71a2a2357aa71e281064c3b8dd7d7ce857241a6b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 12:45:42 +0000 Subject: [PATCH 17/18] fixed bug not pulling out sloping ceiling rec --- recommendations/RoofRecommendations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 5eb81439..71e47ba6 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -1032,7 +1032,7 @@ class RoofRecommendations: """ sloping_ceiling_recommendation = next( - (x for x in non_invasive_recommendations if ["type"] == "sloping_ceiling_insulation"), {} + (x for x in non_invasive_recommendations if x["type"] == "sloping_ceiling_insulation"), {} ) new_description = "Pitched, insulated" From a8d772abfc0733024222a4e8b23f85349ce344c4 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 28 Jan 2026 18:44:03 +0000 Subject: [PATCH 18/18] minor handling for when a user switches off ventilation as an option --- backend/engine/engine.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backend/engine/engine.py b/backend/engine/engine.py index 67353b7a..e833eb89 100644 --- a/backend/engine/engine.py +++ b/backend/engine/engine.py @@ -1051,11 +1051,14 @@ async def model_engine(body: PlanTriggerRequest): property_required_measures = [m for m in recommendations[p.id] if m[0]["type"] in body.required_measures] measures_to_optimise = [m for m in recommendations[p.id] if m[0]["type"] not in body.required_measures] + ventilation_included = "ventilation" in property_measure_types + # If a measure requiring ventilation is selected, and the property does not have ventilation, we enfore # its inclusion + needs_ventilation = any( x in property_measure_types for x in assumptions.measures_needing_ventilation - ) and not p.has_ventilation + ) and not p.has_ventilation and ventilation_included if not measures_to_optimise: # Nothing to do, we just reshape the recommendations