Merge pull request #1266 from Hestia-Homes/feature/hyde_make_it_more_accurate_with_tests

test for 16

.claude/skills/expand-sap-accuracy-corpus/worklist.md

@@ -114,7 +114,54 @@ not-predictable (empty/insufficient cohort) or already fixed by the
 and skipped rate-limited fetches, so a deeper gap could be missed.
 
 **Also done:** `deployment/terraform/lambda/modelling_e2e/variables.tf`
-`maximum_concurrency` default 2 → 4 (per request).
+`maximum_concurrency` default 2 → 4 (per request). Cohort skip-guard widened to
+`(ValueError, AttributeError, KeyError, TypeError)` (mapper-shape errors) so a
+malformed cert (e.g. PV-as-list AttributeError) is skipped+reported too, not just
+missing-field ValueErrors; transient `EpcApiError` (subclasses `Exception`) still
+propagates. +regression test.
+
+### 📋 PLAN — close the 8 modelling_e2e mapping gaps (2026-06-23 run, portfolio 796)
+The 8 failed prediction targets reduce to **5 distinct mapper-gap classes** (the fix
+targets). Per class: fix the mapper GENERICALLY, guard with BOTH the RdSAP-21.0.1
+corpus gauge AND the **component-accuracy gate** (`test_component_accuracy_gate.py`
+— every newly-mappable cert joins the prediction donor pool and can tip near-tie
+similarity, as `multiple_glazed_proportion=100` did: regressed has_hot_water_cylinder
+30→29/36 + door_count 23→25/36 ⇒ REVERTED). Then validate the now-mappable cert via
+`/expand-sap-accuracy-corpus` on **the cohort cert's UPRN** (below), saving
+epc.json + elmhurst_summary.pdf + elmhurst_worksheet.pdf under
+`real_life_examples/<schema>/uprn_<uprn>/` and committing them.
+
+- **P1 — `built_form` missing (SAP-16.0)** — HIGHEST impact (5 certs / 3 targets:
+  10013151061, 100020397529, 100020407755). Cert→UPRN: 8742-…-4926→10070004512,
+  0004-…-7395→100020407745, 2338-…-5950→100020407732, 9328-…-0924→100020407771
+  (8904-…-4623 has NO uprn — validate via one of the others). ⚠ built_form IS a
+  strong prediction similarity feature → highest gate-regression risk: DERIVE it
+  (from `dwelling_type` text / `property_type`), don't flat-default; re-measure the
+  gate. Corpus-validate uprn 10070004512.
+- **P2 — `photovoltaic_supply` lodged as a LIST → AttributeError (`'list' has no
+  attribute none_or_no_details`)** — target 22086690 (BN2 9ZN). NEW real shape bug
+  (not missing-field); escaped the skip net until widened above. Clean fix: coerce
+  the list shape in the PV mapper (like the Measurement-dict co2 fix). Low gate risk.
+  ⚠ FIRST identify the offending cert in the BN2 9ZN cohort (lookup didn't surface it
+  in the first 50 — search the full cohort for `photovoltaic_supply` as list).
+- **P3 — `window` missing, `windows` is a DICT not list (SAP-16.1)** — cert
+  9768-…-7974 → uprn 100062614005, target 100061905741. Clean normaliser fix: handle
+  `windows` as a dict (take it directly) in `_normalize_sap_schema_16_x`. Corpus-
+  validate uprn 100062614005.
+- **P4 — `multiple_glazed_proportion` missing (SAP-16.3)** — cert 0418-…-7970 → uprn
+  22144943, target 22082258. The flat-100 default REGRESSED the gate (reverted).
+  DERIVE it (single→0 / double→100 from `multiple_glazing_type` / window desc) so it
+  doesn't perturb similarity, then re-measure the gate. Corpus-validate uprn 22144943.
+- **P5 — RdSAP-21.0.0 systematic ADR-0028 alignment** (`SapWindow.glazing_gap` +
+  13 top-level fields) — cert 3135-…-2206 → uprn 100021919725, target 100021919718.
+  Align RdSapSchema21_0_0 Optionals to 21.0.1 + review `from_rdsap_schema_21_0_0`
+  None-coalescing. Larger/riskier. Corpus-validate uprn 100021919725.
+- **P6 — `window` + genuinely sparse (SAP-16.2)** — cert 8257-…-4992 → uprn
+  10070009758, target 100020401711. Also omits door_count/habitable/glazed_area →
+  fail-loud is likely CORRECT (insufficient data). DOCUMENT decision; don't force-map.
+
+After all: re-run the modelling_e2e failure sweep to confirm the cohort skips drop
+to zero (or only P6-class), and that no new gate regression slipped in.
 
 - [x] 🔧 10096028301 — SAP-Schema-19.1.0 (full-SAP g/f FLAT, band M, combi PCDB 17929, MEV, AP50 3.5) · eng 82 / elm 82 (lodged 85) · PINNED engine 82. 🔧 mapper added: `from_sap_schema_19_1_0`. Built in Elmhurst (boiler 17929 via search, control CBE/2106, water from primary, MEV on, AP50 Blower Door 3.5+cert). Engine EXACTLY matches Elmhurst worksheet (82.11 vs 82); engine-on-Elmhurst-inputs 82.16 ≈ 82 → calculator faithful. −3 vs lodged = measured-U-vs-RdSAP-default + MEV extract-not-recovery (documented). No mapper change beyond coverage.
 - [x] 🔧 100021943298 — SAP-Schema-16.1 (g/f FLAT, band B, solid-brick internal, combi PCDB 10328) · eng 76 / elm 75 (lodged 72) · PINNED engine 76. 🔧 mapper added: `from_sap_schema_16_1`. Built in Elmhurst (boiler 10328 via search, control CBE/2106, water from primary, wall insulation thickness Unknown); worksheet 75 → engine within ~1 (tightest agreement, reduced-field). Boiler-select + water-heating + control dialogs all driven via automation (two-step row→Select / cascade + coordinate-OK). No mapper change beyond coverage.

deployment/terraform/lambda/modelling_e2e/variables.tf

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

repositories/comparable_properties/epc_comparable_properties_repository.py

@@ -80,18 +80,26 @@ class EpcComparablePropertiesRepository(ComparablePropertiesRepository):
                 cohort.append(comparable)
         return cohort
 
+    # Mapper-shape errors: the cert's lodged data does not fit the schema/mapper.
+    # `ValueError` — missing required field / unmapped code (`UnmappedApiCode`);
+    # `AttributeError` — a field lodged in the wrong shape (e.g. `photovoltaic_supply`
+    # as a list where a `PhotovoltaicSupply` object is expected → "'list' object has
+    # no attribute 'none_or_no_details'"); `KeyError`/`TypeError` — analogous
+    # structural mismatches. Transient API failures (`EpcApiError` &c.) subclass
+    # `Exception` directly, NOT these, so they still propagate and fail the run loud.
+    _UNMAPPABLE_CERT_ERRORS = (ValueError, AttributeError, KeyError, TypeError)
+
     def _comparable_or_skip(
         self, result: EpcSearchResult, coordinates: dict[int, Coordinates]
     ) -> Optional[ComparableProperty]:
         """Map one cohort cert, or record + skip it when the mapper raises a
-        ``ValueError`` (missing required field / unmapped code — see
-        ``UnmappedApiCode``). One unmappable cert must NOT abort the whole cohort;
-        transient API errors are NOT ``ValueError`` and still propagate."""
+        mapper-shape error (see ``_UNMAPPABLE_CERT_ERRORS``). One unmappable cert
+        must NOT abort the whole cohort; transient API errors still propagate."""
         try:
             epc: EpcPropertyData = self._epc_client.get_by_certificate_number(
                 result.certificate_number
             )
-        except ValueError as exc:
+        except self._UNMAPPABLE_CERT_ERRORS as exc:
             self.skipped.append(
                 SkippedCohortCert(
                     certificate_number=result.certificate_number,

tests/repositories/comparable_properties/test_epc_comparable_properties_repository.py

@@ -176,6 +176,31 @@ def test_unmappable_cohort_cert_is_skipped_and_recorded() -> None:
     assert "missing required field 'window'" in repo.skipped[0].error
 
 
+def test_malformed_cert_shape_attributeerror_is_skipped_and_recorded() -> None:
+    # Arrange — a cohort cert lodges a field in the wrong shape (e.g.
+    # `photovoltaic_supply` as a list), so the mapper raises AttributeError
+    # ("'list' object has no attribute 'none_or_no_details'") — a mapper-shape
+    # error like the missing-field ValueErrors, so it must skip + record, not abort.
+    client = _FakeEpcClientRaising(
+        [_result("CERT-OK", uprn=12345), _result("CERT-BAD", uprn=67890)],
+        failures={
+            "CERT-BAD": AttributeError(
+                "'list' object has no attribute 'none_or_no_details'"
+            )
+        },
+    )
+    repo = EpcComparablePropertiesRepository(client, _FakeGeospatial({}))
+
+    # Act
+    candidates = repo.candidates_for("LS6 1AA")
+
+    # Assert — bad cert skipped + recorded; good cert survives.
+    assert [c.certificate_number for c in candidates] == ["CERT-OK"]
+    assert len(repo.skipped) == 1
+    assert repo.skipped[0].certificate_number == "CERT-BAD"
+    assert "none_or_no_details" in repo.skipped[0].error
+
+
 def test_transient_fetch_error_is_not_swallowed_as_unmappable() -> None:
     # Arrange — a non-ValueError (e.g. a transient API failure) must NOT be
     # silently skipped as "unmappable"; it propagates so the run fails loudly.