Merge branch 'main' into feature/batch-save-and-delete-epc

applications/modelling_e2e/handler.py

@@ -166,7 +166,9 @@ class _PropertyWrite:
     scenario_id: int
     is_default: bool
     lodged_epc: Optional[EpcPropertyData]
+    lodged_epc_is_new: bool
     predicted_epc: Optional[EpcPropertyData]
+    predicted_epc_is_new: bool
     spatial: Optional[SpatialReference]
     solar: Optional[_SolarWrite]
     plan: Plan
@@ -187,12 +189,12 @@ def _flush_writes(engine: Engine, writes: list[_PropertyWrite]) -> None:
     lodged_requests = [
         EpcSaveRequest(w.lodged_epc, property_id=w.property_id, portfolio_id=w.portfolio_id, source="lodged")
         for w in writes
-        if w.lodged_epc is not None
+        if w.lodged_epc is not None and w.lodged_epc_is_new
     ]
     predicted_requests = [
         EpcSaveRequest(w.predicted_epc, property_id=w.property_id, portfolio_id=w.portfolio_id, source="predicted")
         for w in writes
-        if w.predicted_epc is not None
+        if w.predicted_epc is not None and w.predicted_epc_is_new
     ]
     with PostgresUnitOfWork(lambda: Session(engine)) as uow:
         if lodged_requests:
@@ -230,7 +232,7 @@ def _get_engine() -> Engine:
         # everything up front through one short-lived read Session, closes it,
         # then writes each Property in a sequential Unit of Work — and the Unit of
         # Work resolves overrides on its own session — so no two Sessions overlap
-        # and a single connection suffices. 32 concurrent containers × 1 = 32
+        # and a single connection suffices. 12 concurrent containers × 1 = 12
         # against RDS.
         #
         # NullPool, not a fixed pool, enforces that as a *graceful* ceiling rather
@@ -407,7 +409,9 @@ def _predict_epc(
     orchestrator_cm=_shared_engine_orchestrator,
     pass_task_orchestrator=True,
 )
-def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator, task_id: UUID) -> None:
+def handler(
+    body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator, task_id: UUID
+) -> None:
     trigger = ModellingE2ETriggerBody.model_validate(body)
     property_ids = trigger.property_ids
     portfolio_id = trigger.portfolio_id
@@ -509,9 +513,7 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
         )
         epc_repo = EpcPostgresRepository(read_session)
         stored_lodged_epcs: dict[int, EpcPropertyData] = (
-            epc_repo.get_for_properties(property_ids)
-            if not refetch_epc
-            else {}
+            epc_repo.get_for_properties(property_ids) if not refetch_epc else {}
         )
         stored_predicted_epcs: dict[int, EpcPropertyData] = (
             epc_repo.get_predicted_for_properties(property_ids)
@@ -534,24 +536,29 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
 
             spatial = _spatial_for(geospatial, uprn)
             restrictions = (
-                spatial.restrictions
-                if spatial is not None
-                else PlanningRestrictions()
+                spatial.restrictions if spatial is not None else PlanningRestrictions()
             )
             coordinates: Optional[Coordinates] = (
                 spatial.coordinates if spatial is not None else None
             )
 
             stored_lodged = stored_lodged_epcs.get(pid)
+            lodged_epc_is_new = False
             if refetch_epc:
                 epc: Optional[EpcPropertyData] = epc_client.get_by_uprn(uprn)
+                lodged_epc_is_new = epc is not None
             elif stored_lodged is not None:
-                logger.info(f"property={pid} using stored lodged EPC (refetch_epc=False)")
+                logger.info(
+                    f"property={pid} using stored lodged EPC (refetch_epc=False)"
+                )
                 epc = stored_lodged
             else:
-                epc = None  # no stored lodged EPC; prediction path handles this property
+                epc = (
+                    None  # no stored lodged EPC; prediction path handles this property
+                )
             overrides = overlays_from(overrides_reader.overrides_for(pid))
             predicted_epc: Optional[EpcPropertyData] = None
+            predicted_epc_is_new = False
 
             if epc is not None:
                 logger.info(f"property={pid} lodged EPC found")
@@ -566,9 +573,7 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
                     landlord_overrides=overrides,
                 ).effective_epc
             else:
-                logger.info(
-                    f"property={pid} no lodged EPC — attempting prediction"
-                )
+                logger.info(f"property={pid} no lodged EPC — attempting prediction")
                 stored_predicted = stored_predicted_epcs.get(pid)
                 if not repredict_epc and stored_predicted is not None:
                     logger.info(
@@ -587,6 +592,7 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
                         broaden=_broaden,
                         predictor=predictor,
                     )
+                    predicted_epc_is_new = True
                 effective_epc = Property(
                     identity=PropertyIdentity(
                         portfolio_id=portfolio_id,
@@ -619,8 +625,7 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
                 print_table=False,
             )
             logger.info(
-                f"property={pid} modelling complete "
-                f"measures={len(plan.measures)}"
+                f"property={pid} modelling complete " f"measures={len(plan.measures)}"
             )
 
             if dry_run:
@@ -649,8 +654,8 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
 
             # Queue this Property's writes rather than committing now — the
             # whole batch is persisted in one Unit of Work after the loop
-            # (see _flush_writes). The EPC is saved in its lodged or predicted
-            # slot (ADR-0031) at flush time depending on which is set here.
+            # (see _flush_writes). The *_is_new flags gate EPC saves so that
+            # EPCs read from DB unchanged are not re-written.
             accumulated.append(
                 _PropertyWrite(
                     property_id=pid,
@@ -659,7 +664,9 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator,
                     scenario_id=scenario_id,
                     is_default=scenario.is_default,
                     lodged_epc=epc,
+                    lodged_epc_is_new=lodged_epc_is_new,
                     predicted_epc=predicted_epc,
+                    predicted_epc_is_new=predicted_epc_is_new,
                     spatial=spatial,
                     solar=solar_write,
                     plan=plan,

datatypes/epc/domain/epc_property_data.py

@@ -370,7 +370,12 @@ class SapEnergySource:
     wind_turbines_terrain_type: str  # int in API, str (e.g. "Suburban") in site notes
     electricity_smart_meter_present: bool
 
-    pv_connection: Optional[Union[int, str]] = None  # int from API; str from site notes
+    # gov-API enum (int): 0 = no PV, 1 = PV present but NOT connected to the
+    # dwelling's own electricity meter (communal / separately metered), 2 = PV
+    # connected to the dwelling's meter. Per RdSAP 10 §11.1 / SAP 10.2 Appendix M,
+    # PV is credited to the dwelling's SAP only when connected (== 2); see
+    # `_pv_connected_to_dwelling_meter` in cert_to_inputs. str from site notes.
+    pv_connection: Optional[Union[int, str]] = None
     photovoltaic_supply: Optional[PhotovoltaicSupply] = None
     photovoltaic_arrays: Optional[List[PhotovoltaicArray]] = None
     wind_turbine_details: Optional[WindTurbineDetails] = None

deployment/terraform/lambda/modelling_e2e/variables.tf

@@ -26,7 +26,7 @@ variable "reserved_concurrent_executions" {
 
 variable "maximum_concurrency" {
   type        = number
-  default     = 32
+  default     = 12
   description = "Maximum concurrent Lambda invocations from the SQS trigger."
 }
 

deployment/terraform/shared/main.tf

@@ -81,6 +81,10 @@ resource "aws_db_instance" "default" {
   publicly_accessible = true
   # Specify the CA certificate with the default RDS CA certificate
   ca_cert_identifier = "rds-ca-rsa2048-g1"
+
+  # Performance Insights (7-day retention is free)
+  performance_insights_enabled          = true
+  performance_insights_retention_period = 7
   # Temporary to enfore immediate change
   apply_immediately = true
   # Set up storage type to gp3 for better performance

domain/modelling/generators/solar_recommendation.py

@@ -49,9 +49,13 @@ _BATTERY_CAPACITY_KWH = 5.0
 # Watts → kilowatts for peak-power.
 _WATTS_PER_KW = 1000.0
 # The dwelling's PV connects to its own meter (the after-cert §19 "Connected to
-# the dwelling's meter: Yes"). Non-load-bearing for the SAP cascade; carried for
-# fidelity. 1 = connected, the modal install case.
-_PV_CONNECTED_TO_DWELLING = 1
+# the dwelling's meter: Yes"). LOAD-BEARING: `cert_to_inputs` credits PV to the
+# dwelling's SAP only when `pv_connection == 2` ("connected"); value 1 means
+# "present but NOT connected" and zeroes the credit. gov-API enum (corpus- and
+# Elmhurst-validated): 0 = no PV, 1 = not connected, 2 = connected (the modal
+# install case, 52 vs 5 on the RdSAP-21.0.1 corpus). A newly-installed
+# recommended array is connected to the dwelling's own meter.
+_PV_CONNECTED_TO_DWELLING = 2
 
 # A roof plane within this many degrees of due north (0°/360°, Google compass
 # convention) is dropped: it generates little and is not worth panelling. The

domain/sap10_calculator/rdsap/cert_to_inputs.py

@@ -3156,6 +3156,42 @@ def _pv_array_generation_kwh_per_yr(
     return _PV_MODULE_EFFICIENCY_FACTOR * array.peak_power * s * z
 
 
+# gov-API `sap_energy_source.pv_connection` enum (RdSAP 10 §11.1 /
+# SAP 10.2 Appendix M — "PV is included in the dwelling's assessment only
+# if connected to the dwelling's electricity meter"):
+#   0 = no PV
+#   1 = PV present but NOT connected to the dwelling's own meter
+#   2 = PV connected to the dwelling's own meter
+# Validated on the RdSAP-21.0.1 corpus (57 PV certs): pv_connection=1 certs
+# reconcile to the lodged SAP only WITHOUT a credit (MAE 4.48→1.22, 0/5 need
+# it); pv_connection=2 certs need it (MAE 0.98 vs 10.29 without). Accredited
+# Elmhurst proof: identical dwelling = SAP 87 connected vs SAP 74 not.
+_PV_CONNECTION_CONNECTED_TO_DWELLING_METER: Final[int] = 2
+
+
+def _pv_connected_to_dwelling_meter(epc: EpcPropertyData) -> bool:
+    """Whether a lodged PV array may be credited to this dwelling's SAP, i.e.
+    whether it is connected to the dwelling's own electricity meter.
+
+    Keyed on the gov-API integer `pv_connection`: only value 2 ("connected")
+    earns a credit; value 1 ("present but not connected" — a communal /
+    separately-metered array) contributes nothing to the dwelling's energy
+    cost, CO2 or primary energy, per RdSAP 10 §11.1 / SAP 10.2 Appendix M.
+
+    A non-integer `pv_connection` (None, or the site-notes `str` form which
+    does not yet capture the connection flag) is NOT a determinate
+    "not connected" signal, so it preserves the existing credit-if-array
+    behaviour — no regression on the Elmhurst/Summary path or synthetic
+    CalculatorInputs. The Elmhurst extractor parses "Connected to the
+    dwelling's meter" today only as a parse stop-token; capturing its value
+    is a follow-up that would let this gate apply to that path too.
+    """
+    pv_connection = epc.sap_energy_source.pv_connection
+    if isinstance(pv_connection, int):
+        return pv_connection == _PV_CONNECTION_CONNECTED_TO_DWELLING_METER
+    return True
+
+
 def _pv_generation_kwh_per_yr(
     epc: EpcPropertyData,
     climate: "int | PostcodeClimate",
@@ -3170,7 +3206,13 @@ def _pv_generation_kwh_per_yr(
     roof area" PV figure (no detailed kWp): synthesize a single PV
     array with kWp = 0.12 × PV area, South orientation, 30° pitch,
     Modest overshading.
+
+    Returns 0 when the array is not connected to the dwelling's own meter
+    (`_pv_connected_to_dwelling_meter` — gov-API `pv_connection=1`), per
+    RdSAP 10 §11.1 / SAP 10.2 Appendix M.
     """
+    if not _pv_connected_to_dwelling_meter(epc):
+        return 0.0
     arrays = epc.sap_energy_source.photovoltaic_arrays
     if not arrays:
         arrays = _synthesize_pv_arrays_from_percent_roof_area(epc)
@@ -3217,7 +3259,13 @@ def _pv_monthly_generation_kwh(
 ) -> tuple[float, ...]:
     """SAP 10.2 Appendix M1 §2 (p.92) — monthly E_PV summed across all
     PV arrays. Annual sum matches `_pv_generation_kwh_per_yr` to
-    float precision."""
+    float precision.
+
+    Returns all-zero when the array is not connected to the dwelling's own
+    meter (`_pv_connected_to_dwelling_meter`), so the §10a cost split and the
+    CO2 / PE cascades all see no PV — mirroring the annual helper's gate."""
+    if not _pv_connected_to_dwelling_meter(epc):
+        return (0.0,) * 12
     arrays = epc.sap_energy_source.photovoltaic_arrays
     if not arrays:
         arrays = _synthesize_pv_arrays_from_percent_roof_area(epc)

domain/sap10_ml/tests/_fixtures.py

@@ -267,6 +267,7 @@ def make_minimal_sap10_epc(
     sap_heating: Optional[SapHeating] = None,
     photovoltaic_arrays: Optional[list[PhotovoltaicArray]] = None,
     photovoltaic_supply_percent_roof_area: Optional[int] = None,
+    pv_connection: Optional[int] = None,
     mains_gas: bool = True,
     electricity_smart_meter_present: bool = False,
     gas_smart_meter_present: bool = False,
@@ -308,6 +309,7 @@ def make_minimal_sap10_epc(
         sap_energy_source=SapEnergySource(
             mains_gas=mains_gas,
             meter_type="Single",
+            pv_connection=pv_connection,
             pv_battery_count=pv_battery_count,
             wind_turbines_count=wind_turbines_count,
             gas_smart_meter_present=gas_smart_meter_present,

scripts/trigger_modelling_e2e_sqs.py

@@ -43,15 +43,15 @@ COMPLETED_SINCE: datetime | None = datetime(
 DRY_RUN: bool = False
 
 # False → Lambda skips the Google Solar fetch (re-uses stored Solar data).
-REFETCH_SOLAR: bool = True
+REFETCH_SOLAR: bool = False
 
 # False → use stored lodged EPC for properties that have one; properties with no
 # stored lodged EPC are treated as EPC-less and routed to prediction (no API call).
-REFETCH_EPC: bool = True
+REFETCH_EPC: bool = False
 
 # False → use stored predicted EPC for EPC-less properties that have one; live
 # prediction still runs when no stored predicted EPC exists for the property.
-REPREDICT_EPC: bool = True
+REPREDICT_EPC: bool = False
 # ---------------------------------------------------------------------------
 
 _REPO_ROOT = Path(__file__).resolve().parents[1]

tests/domain/modelling/test_solar_recommendation.py

@@ -89,6 +89,10 @@ def test_each_option_overlay_installs_per_segment_arrays_and_ensures_export() ->
         assert overlay is not None
         assert overlay.is_dwelling_export_capable is True
         assert overlay.pv_diverter_present is True
+        # A newly-installed recommended array is connected to the dwelling's own
+        # meter, so it must be tagged pv_connection=2 ("connected") — the value
+        # the SAP cascade credits. (1 = present-but-not-connected → zero credit.)
+        assert overlay.pv_connection == 2
         arrays = overlay.photovoltaic_arrays
         assert arrays is not None and len(arrays) >= 1
         assert all(isinstance(a, PhotovoltaicArray) for a in arrays)

tests/domain/sap10_calculator/rdsap/test_cert_to_inputs.py

@@ -40,6 +40,7 @@ from domain.sap10_ml.tests._fixtures import (
     make_floor_dimension,
     make_main_heating_detail,
     make_minimal_sap10_epc,
+    make_pv_array,
     make_sap_heating,
     make_window,
 )
@@ -8387,3 +8388,38 @@ def test_heat_pump_water_scop_not_applied_to_separate_immersion_dhw() -> None:
     # heat pump or a gas boiler (the HP water SCOP does not apply to it).
     assert hp_fuel > 0.0
     assert abs(hp_fuel - boiler_fuel) <= 1e-6
+
+
+def test_pv_credited_only_when_connected_to_dwelling_meter_per_pv_connection() -> None:
+    # RdSAP 10 §11.1 / SAP 10.2 Appendix M: PV-generated electricity is
+    # included in a dwelling's assessment ONLY IF the array is connected to
+    # the dwelling's own electricity meter; an unconnected (communal /
+    # separately-metered) array contributes nothing to the dwelling's energy
+    # cost, CO2 or primary energy. The gov-API `sap_energy_source.pv_connection`
+    # enum encodes this: 0 = no PV, 1 = present but NOT connected, 2 = connected.
+    #
+    # Validated on the RdSAP-21.0.1 corpus (57 PV certs): every pv_connection=1
+    # cert reconciles BETTER without the credit (MAE 4.48 -> 1.22, 0/5 need it),
+    # while pv_connection=2 certs need it (MAE 0.98 vs 10.29 without). Accredited
+    # Elmhurst proof: an identical dwelling rates SAP 87 with "Connected to
+    # Dwelling = Yes" (credit -£167) vs SAP 74 with "No" (credit £0).
+    array = [make_pv_array(peak_power=3.0)]
+
+    def _gen(pv_connection: int) -> float:
+        epc = make_minimal_sap10_epc(
+            dwelling_type="Mid-terrace house",
+            total_floor_area_m2=70.0,
+            habitable_rooms_count=3,
+            country_code="ENG",
+            photovoltaic_arrays=array,
+            is_dwelling_export_capable=True,
+            pv_connection=pv_connection,
+        )
+        return cert_to_inputs(epc).pv_generation_kwh_per_yr
+
+    # pv_connection=2 (connected to the dwelling's meter) → PV serves the
+    # dwelling and is credited.
+    assert _gen(2) > 0.0
+    # pv_connection=1 (present but NOT connected to the dwelling's meter) →
+    # the array contributes nothing to this dwelling's SAP.
+    assert _gen(1) == 0.0

tests/infrastructure/epc_client/test_sap_accuracy_corpus.py

@@ -193,7 +193,7 @@ _CORPUS = Path(
 # within-0.5 71.6% -> 72.5%, MAE 0.819 -> 0.815. Surfaced by Khalim's Elmhurst
 # stress worksheet (simulated case 46): closed its last ventilation residual
 # (our Jan ACH 9.14 -> 9.0748 exact; SAP 29 -> 30 = accredited Elmhurst).
-_MIN_WITHIN_HALF_SAP = 0.73
+_MIN_WITHIN_HALF_SAP = 0.74
 # 0.793 -> 0.789 via the §12 Unknown-meter + dual-electric-immersion off-peak
 # trigger (RdSAP 10 PDF p.62): Apartment 241 (main 691 + 903 dual immersion)
 # -5.38 -> -1.05. Worksheet-validated on "simulated case 48" (Elmhurst SAP 57,
@@ -238,7 +238,17 @@ _MIN_WITHIN_HALF_SAP = 0.73
 # already computes 74. roof_insulation_location="ND" ⟺ party ceiling separates
 # the corpus classes with zero disagreement (all 190 party flats lodge "ND");
 # the 4 mid/ground-floor flats this exposes all move toward lodged, 0 away.
-_MAX_SAP_MAE = 0.762
+# Then 0.761 -> 0.740 (within-0.5 73.6% -> 74.1%) via the PV dwelling-meter
+# connection gate (RdSAP 10 §11.1 / SAP 10.2 Appendix M): PV is credited to the
+# dwelling only when gov-API `pv_connection == 2` ("connected to the dwelling's
+# meter"); == 1 ("present but NOT connected" — communal / separately metered)
+# now contributes zero to cost/CO2/PE. All 5 pv_connection=1 PV certs move
+# inside ±0.5 (e.g. 100051118081 +6.5 -> +0.5); pv_connection=2 certs (52) keep
+# their credit (corpus MAE 0.98 with vs 10.29 without). Khalim's Elmhurst proof:
+# an identical dwelling rates SAP 87 with "Connected to Dwelling = Yes" (credit
+# -£167) vs SAP 74 with "No" (credit £0). Enum decoded empirically: 0 = no PV,
+# 1 = not connected, 2 = connected (the gov-API does not expose it elsewhere).
+_MAX_SAP_MAE = 0.740
 _MAX_CO2_MAE_TONNES = 0.09      # t CO2 / yr vs co2_emissions_current
 _MAX_PE_PER_M2_MAE = 3.5        # kWh / m2 / yr vs energy_consumption_current