mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-07-27 23:35:01 +00:00
Report missing baselines once per batch, and seed the harness's own 🟩
The modelling_e2e handler models one property per run_modelling call, so a per-property "no persisted baseline" line would be one per property across a whole re-model. The harness has no Baseline stage by design and cannot drift from one, so it now establishes its own the way the Baseline orchestrator would; the Postgres-backed lane is where the assertion has something to catch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
29157811e0
commit
223ef7c196
3 changed files with 74 additions and 21 deletions
|
|
@ -29,6 +29,10 @@ from domain.modelling.recommendation import Recommendation
|
|||
from domain.modelling.scenario import Scenario
|
||||
from domain.modelling.solar_potential import SolarPotential
|
||||
from domain.property.property import Property, PropertyIdentity
|
||||
from domain.property_baseline.performance import Performance
|
||||
from domain.property_baseline.property_baseline_performance import (
|
||||
PropertyBaselinePerformance,
|
||||
)
|
||||
from domain.property_baseline.rebaseliner import StubRebaseliner
|
||||
from domain.sap10_calculator.calculator import Sap10Calculator
|
||||
from harness.plan_table import format_plan_table
|
||||
|
|
@ -49,6 +53,7 @@ from repositories.product.product_repository import ProductRepository
|
|||
from tests.orchestration.fakes import (
|
||||
FakeEpcRepo,
|
||||
FakePlanRepository,
|
||||
FakePropertyBaselineRepo,
|
||||
FakePropertyRepo,
|
||||
FakeScenarioRepository,
|
||||
FakeSolarRepo,
|
||||
|
|
@ -230,7 +235,26 @@ def run_modelling(
|
|||
)
|
||||
},
|
||||
)
|
||||
# The Baseline stage this harness has no database for. Modelling asserts its
|
||||
# re-score agrees with the persisted Effective Performance (ADR-0066), so
|
||||
# establish it here the way `PropertyBaselineOrchestrator` would — from the
|
||||
# calculator on this same EPC. Drift is structurally impossible in this lane
|
||||
# (one process, one code version, one picture), so the assertion is a no-op;
|
||||
# seeding it keeps the check honest instead of reporting "no baseline" once
|
||||
# per Property across a whole re-model batch. ~1% of a run_modelling call.
|
||||
baseline_repo = FakePropertyBaselineRepo()
|
||||
baseline_repo.save(
|
||||
PropertyBaselinePerformance(
|
||||
lodged=None,
|
||||
effective=Performance.from_sap_result(Sap10Calculator().calculate(epc)),
|
||||
rebaseline_reason="physical_state_changed",
|
||||
space_heating_kwh=0.0,
|
||||
water_heating_kwh=0.0,
|
||||
),
|
||||
_PROPERTY_ID,
|
||||
)
|
||||
unit = FakeUnitOfWork(
|
||||
property_baseline=baseline_repo,
|
||||
property=property_repo,
|
||||
solar=FakeSolarRepo(
|
||||
by_uprn={_UPRN: solar_insights}
|
||||
|
|
|
|||
|
|
@ -123,6 +123,11 @@ class ModellingOrchestrator:
|
|||
# Resolve Fuel Rates once and reuse the BillDerivation across the batch,
|
||||
# so every baseline/post bill is priced at the same snapshot (ADR-0014).
|
||||
bill_derivation = BillDerivation(self._fuel_rates.get_current())
|
||||
# Properties the Baseline stage never established. Collected and reported
|
||||
# once at the end of the batch — a whole batch can legitimately have none
|
||||
# (the database-free harness), and one line each would bury the real
|
||||
# divergence errors under a duplicate per Property.
|
||||
without_baseline: list[int] = []
|
||||
with self._unit_of_work() as uow:
|
||||
properties = uow.property.get_many(property_ids)
|
||||
scenarios: list[Scenario] = uow.scenario.get_many(scenario_ids)
|
||||
|
|
@ -154,11 +159,12 @@ class ModellingOrchestrator:
|
|||
# baseline Score, which is already computed, rather than
|
||||
# paying for another whole-dwelling re-score per Property.
|
||||
if not checked_baseline:
|
||||
_check_baseline_coherence(
|
||||
if _check_baseline_coherence(
|
||||
property_id,
|
||||
plan.baseline.sap_continuous,
|
||||
uow.property_baseline.get_for_property(property_id),
|
||||
)
|
||||
):
|
||||
without_baseline.append(property_id)
|
||||
checked_baseline = True
|
||||
uow.plan.save(
|
||||
plan,
|
||||
|
|
@ -174,6 +180,14 @@ class ModellingOrchestrator:
|
|||
uow.property.mark_modelled(
|
||||
property_id, has_recommendations=has_recommendations
|
||||
)
|
||||
if without_baseline:
|
||||
logger.warning(
|
||||
"%s of %s properties had no persisted Baseline Performance "
|
||||
"and were modelled against their own re-score; first ids: %s",
|
||||
len(without_baseline),
|
||||
len(property_ids),
|
||||
without_baseline[:10],
|
||||
)
|
||||
uow.commit()
|
||||
|
||||
def _plan_for(
|
||||
|
|
@ -517,7 +531,7 @@ def _check_baseline_coherence(
|
|||
property_id: int,
|
||||
modelled_baseline_sap: float,
|
||||
persisted: Optional[PropertyBaselinePerformance],
|
||||
) -> None:
|
||||
) -> bool:
|
||||
"""Assert that Modelling and the Baseline stage agree on what the Property
|
||||
scores now, and say so loudly when they do not (ADR-0002, #1655).
|
||||
|
||||
|
|
@ -535,20 +549,21 @@ def _check_baseline_coherence(
|
|||
a real defect upstream (see #1656) and should be investigated, not papered
|
||||
over.
|
||||
|
||||
A missing row is a data gap, not a modelling fault: warn and carry on, so it
|
||||
degrades one Property rather than aborting the batch (ADR-0012 reserves the
|
||||
abort for a load-bearing calculator raise)."""
|
||||
A missing row is a data gap, not a modelling fault, so it is **returned**
|
||||
(``True``) for the caller to aggregate rather than logged here: it degrades
|
||||
one Property instead of aborting the batch (ADR-0012 reserves the abort for a
|
||||
load-bearing calculator raise), and an entire batch can legitimately have no
|
||||
Baseline stage at all — the database-free harness the ``modelling_e2e``
|
||||
handler runs on has none, so a per-Property line would bury the real errors
|
||||
under one duplicate per Property.
|
||||
|
||||
A divergence stays per-Property: it is rare, and the Property is the thing
|
||||
you need to go and look at."""
|
||||
if persisted is None:
|
||||
logger.warning(
|
||||
"no persisted Baseline Performance for property_id=%s; modelling "
|
||||
"against its own re-score (modelled_baseline_sap=%.2f)",
|
||||
property_id,
|
||||
modelled_baseline_sap,
|
||||
)
|
||||
return
|
||||
return True
|
||||
effective_sap: int = persisted.effective.sap_score
|
||||
if abs(modelled_baseline_sap - effective_sap) <= _MAX_BASELINE_DIVERGENCE:
|
||||
return
|
||||
return False
|
||||
logger.error(
|
||||
"modelling baseline diverges from persisted Effective Performance for "
|
||||
"property_id=%s: modelled=%.2f effective=%s (%s). Plan post_* figures "
|
||||
|
|
@ -559,6 +574,7 @@ def _check_baseline_coherence(
|
|||
effective_sap,
|
||||
persisted.rebaseline_reason,
|
||||
)
|
||||
return False
|
||||
|
||||
|
||||
def _target_sap(scenario: Scenario) -> Optional[float]:
|
||||
|
|
|
|||
|
|
@ -81,18 +81,31 @@ def test_a_baseline_agreeing_within_rounding_is_silent(
|
|||
assert caplog.records == []
|
||||
|
||||
|
||||
def test_a_property_with_no_persisted_baseline_warns_rather_than_failing(
|
||||
def test_a_property_with_no_persisted_baseline_is_reported_not_logged(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
# Arrange — the Property never went through the Baseline stage. A data gap
|
||||
# should degrade one Property, not abort the batch (ADR-0012 reserves the
|
||||
# abort for a load-bearing calculator raise).
|
||||
# abort for a load-bearing calculator raise) — but it must not log per
|
||||
# Property either: whole batches legitimately have no Baseline stage (the
|
||||
# database-free harness the modelling_e2e handler runs on), and one line per
|
||||
# Property buries the real errors in tens of thousands of duplicates.
|
||||
|
||||
# Act
|
||||
with caplog.at_level(logging.WARNING):
|
||||
_check_baseline_coherence(749719, 63.22, None)
|
||||
missing = _check_baseline_coherence(749719, 63.22, None)
|
||||
|
||||
# Assert — visible, but not an error and not a raise.
|
||||
assert "749719" in caplog.text
|
||||
assert any(r.levelno == logging.WARNING for r in caplog.records)
|
||||
assert not any(r.levelno >= logging.ERROR for r in caplog.records)
|
||||
# Assert — reported back to the caller to aggregate, silent in itself.
|
||||
assert missing is True
|
||||
assert caplog.records == []
|
||||
|
||||
|
||||
def test_a_property_with_a_baseline_is_not_reported_as_missing() -> None:
|
||||
# Arrange
|
||||
persisted: Optional[PropertyBaselinePerformance] = _persisted(63)
|
||||
|
||||
# Act
|
||||
missing = _check_baseline_coherence(749719, 63.22, persisted)
|
||||
|
||||
# Assert
|
||||
assert missing is False
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue