Model/docs/PR_NOTE_system_built_basement_1177.md
Khalim Conn-Kowlessar 86a725224b docs: PR note for #1177 — system_build property vs field merge collision
Flags the SY/B disambiguation change and the field-vs-property merge
landmine (raises AttributeError at first EpcPropertyData instantiation,
not at import; git merges silently) for the feature/bill-derivation
reviewer, with the recommended reconciliation and the strict-xfail
tripwire they own.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-04 19:51:29 +00:00

3.3 KiB

PR note — SY system-built vs B basement wall (issue #1177)

For the reviewer / the feature/bill-derivation session. Fold the relevant parts into the PR description; delete this file before/at merge.

What this branch changed (commit bd25a3c7)

wall_construction == 6 (WALL_SYSTEM_BUILT) is the canonical wall type for system-built — and it stays there. The basement signal, which had hijacked code 6, is moved onto a dedicated flag:

  • SapBuildingPart.wall_is_basement: Optional[bool] and SapAlternativeWall.is_basement: Optional[bool].
  • main_wall_is_basement / is_basement_wall honour the flag when set, else fall back to the legacy wall_construction == 6 heuristic (so untouched API basements and the cert 000565 "B" cohort are unchanged).
  • Elmhurst: _elmhurst_wall_is_basement sets the flag from the distinct "SY"/"B" labels (SY→False, B→True, other→None).
  • gov-EPC API: from_api_response post-processes via _clear_basement_flag_when_system_built — when the cert addendum.system_build is True, the code-6 basement heuristic is cleared.
  • EpcPropertyData.system_build is a derived @property (not a stored field): the MAIN wall is system-built iff wall_construction == 6 and it is not flagged basement. (Per the call: "system build is the wall type and should be on wall_construction.")

Acceptance verified: system-built main wall → wall_construction == 6, main_wall_is_basement is False, system_build is True; genuine basement main wall → main_wall_is_basement is True, system_build is False. Full §4 suite green (2404 passed), zero new pyright errors.

⚠️ Merge collision: system_build field (yours) vs property (this branch)

The #1177 prompt referenced EpcPropertyData.system_build as an existing field on feature/bill-derivation. This branch adds system_build as a @property. They share the name but live in different regions of the class, so:

  • Git will likely merge them silently (no textual conflict).
  • Python will NOT raise at import — the class defines fine.
  • It raises AttributeError: property 'system_build' ... has no setter at the first EpcPropertyData(...) instantiation — i.e. the first mapper call, so the test suite fails immediately. (Reproduced.)

So the collision is caught fast but is a landmine, not a clean signal. Resolve it deliberately at merge — pick ONE representation:

  • Recommended (matches the agreed model): drop the stored field; keep the derived property (system-built is the wall type). If your code currently assigns epc.system_build = …, replace those writes with setting the underlying wall type / basement flag, or add a setter.
  • Or keep your stored field and delete this branch's property — but then populate the field consistently with the wall type on BOTH paths (API addendum and Elmhurst "SY"), so system_build and wall_construction/the basement flag never disagree.

Tripwire you own

tests/domain/modelling/test_elmhurst_cascade_pins.py::test_system_built_generator_offers_ewi_and_iwi_each_pinning_its_after is a strict-xfail on your branch (fixtures system_built_{ewi,iwi}_001431_{before,after}.pdf are committed there, not here). With this fix the behaviour it guards is satisfied, so it should flip xfail→xpass — delete the marker when it does.