mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-08-03 05:18:22 +00:00
Refresh a property's default Plan when its owning scenario is re-run
A Plan became default only when its Scenario was flagged default or the property had no Plan yet. Scenarios are created is_default=False, so in practice only the first-ever Plan was ever promoted — re-modelling a property appended a new Plan but left the default frozen on the first run, so a re-model never surfaced on the FE (property 792168 kept its 2026-07-22 scenario-1328 default through three same-scenario re-runs). Also promote when the re-run models the SAME scenario that currently owns the property's default, so a re-run refreshes the shown Plan in place (the save demotes the stale one). A re-run of a different scenario still does not hijack the default. New PlanRepository.default_plan_scenario_by_property surfaces each property's default scenario for the decision. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
035d2f5136
commit
0ca8ca31a0
6 changed files with 213 additions and 12 deletions
|
|
@ -241,13 +241,18 @@ def _flush_writes(engine: Engine, writes: list[_PropertyWrite]) -> None:
|
|||
uow.epc.save_batch(lodged_requests)
|
||||
if predicted_requests:
|
||||
uow.epc.save_batch(predicted_requests)
|
||||
# First plan wins: a property with no default Plan yet gets this Plan
|
||||
# as its default whatever the Scenario's flag says (the legacy
|
||||
# p.is_new rule — readers select one default Plan per property, so a
|
||||
# first run under a non-default Scenario must still produce one).
|
||||
# Also heals properties left default-less by runs before this rule.
|
||||
pids_with_default: set[int] = uow.plan.property_ids_with_default_plans(
|
||||
[w.property_id for w in writes]
|
||||
# A Plan becomes this property's default when any of:
|
||||
# - the Scenario is flagged default (w.is_default);
|
||||
# - the property has no default Plan yet (first run — readers select one
|
||||
# default per property, so a first run under a non-default Scenario must
|
||||
# still produce one; also heals default-less properties);
|
||||
# - this run re-models the SAME Scenario that currently owns the default,
|
||||
# so a re-run refreshes the shown Plan in place (the save demotes the
|
||||
# stale one). Without this, and because scenarios are created
|
||||
# is_default=False, the first-ever Plan stays default forever and a
|
||||
# re-model never surfaces (property 792168 / portfolio 850).
|
||||
default_scenario_by_pid: dict[int, int] = (
|
||||
uow.plan.default_plan_scenario_by_property([w.property_id for w in writes])
|
||||
)
|
||||
plan_requests = [
|
||||
PlanSaveRequest(
|
||||
|
|
@ -255,7 +260,11 @@ def _flush_writes(engine: Engine, writes: list[_PropertyWrite]) -> None:
|
|||
property_id=w.property_id,
|
||||
scenario_id=w.scenario_id,
|
||||
portfolio_id=w.portfolio_id,
|
||||
is_default=w.is_default or w.property_id not in pids_with_default,
|
||||
is_default=(
|
||||
w.is_default
|
||||
or w.property_id not in default_scenario_by_pid
|
||||
or default_scenario_by_pid[w.property_id] == w.scenario_id
|
||||
),
|
||||
)
|
||||
for w in writes
|
||||
]
|
||||
|
|
|
|||
|
|
@ -59,6 +59,26 @@ class PlanPostgresRepository(PlanRepository):
|
|||
).all()
|
||||
return {int(row) for row in rows}
|
||||
|
||||
def default_plan_scenario_by_property(
|
||||
self, property_ids: list[int]
|
||||
) -> dict[int, int]:
|
||||
"""Each property's current default Plan's scenario_id (absent when it has
|
||||
no default). One default per property (the demote invariant), so the map
|
||||
is well-defined."""
|
||||
if not property_ids:
|
||||
return {}
|
||||
rows = self._session.exec(
|
||||
select(col(PlanModel.property_id), col(PlanModel.scenario_id)).where(
|
||||
col(PlanModel.property_id).in_(property_ids),
|
||||
col(PlanModel.is_default).is_(True),
|
||||
)
|
||||
).all()
|
||||
return {
|
||||
int(property_id): int(scenario_id)
|
||||
for property_id, scenario_id in rows
|
||||
if scenario_id is not None
|
||||
}
|
||||
|
||||
def save_batch(self, requests: list[PlanSaveRequest]) -> list[int]:
|
||||
"""Persist all Plans in three statements regardless of batch size.
|
||||
|
||||
|
|
|
|||
|
|
@ -63,3 +63,19 @@ class PlanRepository(ABC):
|
|||
its default whatever the Scenario says (the legacy ``p.is_new`` rule —
|
||||
readers need one default Plan per property)."""
|
||||
...
|
||||
|
||||
@abstractmethod
|
||||
def default_plan_scenario_by_property(
|
||||
self, property_ids: list[int]
|
||||
) -> dict[int, int]:
|
||||
"""Each property's current default Plan's ``scenario_id`` (absent when it
|
||||
has no default). One default Plan per property (invariant), so the map is
|
||||
well-defined.
|
||||
|
||||
Lets the orchestrator refresh the default in place: a re-run of the
|
||||
scenario that currently owns the default promotes its fresh Plan (the repo
|
||||
demotes the stale one), while a re-run of a different scenario adds history
|
||||
without hijacking the default. Needed because scenarios are created
|
||||
``is_default=False``, so the Scenario flag never promotes a re-run and the
|
||||
first-Plan-ever default would otherwise be frozen forever."""
|
||||
...
|
||||
|
|
|
|||
|
|
@ -561,10 +561,16 @@ def test_first_plan_for_a_property_becomes_its_default() -> None:
|
|||
)
|
||||
MockUoW = stack.enter_context(patch("applications.modelling_e2e.handler.PostgresUnitOfWork"))
|
||||
mock_uow = MagicMock()
|
||||
mock_uow.plan.property_ids_with_default_plans.return_value = {pid_has_default}
|
||||
MockUoW.return_value.__enter__.return_value = mock_uow
|
||||
MockUoW.return_value.__exit__.return_value = False
|
||||
|
||||
# pid_has_default's current default is for a DIFFERENT scenario than the
|
||||
# one being modelled (SCENARIO_ID), so it is not refreshed and keeps the
|
||||
# non-default flag; pid_first has no default at all.
|
||||
mock_uow.plan.default_plan_scenario_by_property.return_value = {
|
||||
pid_has_default: SCENARIO_ID + 1
|
||||
}
|
||||
|
||||
# Act
|
||||
from applications.modelling_e2e.handler import handler
|
||||
handler.__wrapped__( # type: ignore[attr-defined]
|
||||
|
|
@ -581,6 +587,120 @@ def test_first_plan_for_a_property_becomes_its_default() -> None:
|
|||
assert by_pid[pid_has_default].is_default is False
|
||||
|
||||
|
||||
def test_re_running_the_scenario_that_holds_the_default_refreshes_it() -> None:
|
||||
"""A re-run of the scenario that currently owns the property's default Plan
|
||||
promotes the fresh Plan to default (the repo demotes the stale one), so a
|
||||
re-model surfaces on the FE. Provenance: property 792168 kept its 2026-07-22
|
||||
scenario-1328 plan default through three same-scenario re-runs."""
|
||||
# Arrange — the property's default is for the SAME scenario being modelled.
|
||||
pid = 111
|
||||
mock_engine = _engine_mock([pid], [UPRN], [POSTCODE])
|
||||
scenario = MagicMock()
|
||||
scenario.is_default = False
|
||||
mock_uow = MagicMock()
|
||||
mock_uow.plan.default_plan_scenario_by_property.return_value = {pid: SCENARIO_ID}
|
||||
|
||||
with ExitStack() as stack:
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.os.environ", _ENV))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._get_engine", return_value=mock_engine)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.EpcClientService")
|
||||
).return_value.get_by_uprn.return_value = MagicMock()
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.GeospatialS3Repository"))
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.GoogleSolarApiClient"))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._spatial_for", return_value=None)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._solar_insights_for", return_value=None)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.overlays_from", return_value=[])
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.PropertyOverridesPostgresReader")
|
||||
).return_value.overrides_for_many.return_value = {}
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.ScenarioPostgresRepository")
|
||||
).return_value.get_many.return_value = [scenario]
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.catalogue_snapshot_with_off_catalogue_overrides"))
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.Session"))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.run_modelling", return_value=_plan_mock())
|
||||
)
|
||||
MockUoW = stack.enter_context(patch("applications.modelling_e2e.handler.PostgresUnitOfWork"))
|
||||
MockUoW.return_value.__enter__.return_value = mock_uow
|
||||
MockUoW.return_value.__exit__.return_value = False
|
||||
|
||||
# Act
|
||||
_call_handler(
|
||||
{"property_ids": [pid], "portfolio_id": PORTFOLIO_ID,
|
||||
"scenario_id": SCENARIO_ID, "refetch_solar": False, "dry_run": False}
|
||||
)
|
||||
|
||||
# Assert — the fresh plan for the default-holding scenario is promoted.
|
||||
plan_requests = mock_uow.plan.save_batch.call_args.args[0]
|
||||
assert plan_requests[0].is_default is True
|
||||
|
||||
|
||||
def test_re_running_a_different_scenario_does_not_steal_the_default() -> None:
|
||||
"""A re-run of a scenario OTHER than the one holding the default does not
|
||||
hijack it — the shown scenario only changes via scenario.is_default."""
|
||||
# Arrange — the property's default is for a different scenario.
|
||||
pid = 111
|
||||
mock_engine = _engine_mock([pid], [UPRN], [POSTCODE])
|
||||
scenario = MagicMock()
|
||||
scenario.is_default = False
|
||||
mock_uow = MagicMock()
|
||||
mock_uow.plan.default_plan_scenario_by_property.return_value = {pid: SCENARIO_ID + 1}
|
||||
|
||||
with ExitStack() as stack:
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.os.environ", _ENV))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._get_engine", return_value=mock_engine)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.EpcClientService")
|
||||
).return_value.get_by_uprn.return_value = MagicMock()
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.GeospatialS3Repository"))
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.GoogleSolarApiClient"))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._spatial_for", return_value=None)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler._solar_insights_for", return_value=None)
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.overlays_from", return_value=[])
|
||||
)
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.PropertyOverridesPostgresReader")
|
||||
).return_value.overrides_for_many.return_value = {}
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.ScenarioPostgresRepository")
|
||||
).return_value.get_many.return_value = [scenario]
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.catalogue_snapshot_with_off_catalogue_overrides"))
|
||||
stack.enter_context(patch("applications.modelling_e2e.handler.Session"))
|
||||
stack.enter_context(
|
||||
patch("applications.modelling_e2e.handler.run_modelling", return_value=_plan_mock())
|
||||
)
|
||||
MockUoW = stack.enter_context(patch("applications.modelling_e2e.handler.PostgresUnitOfWork"))
|
||||
MockUoW.return_value.__enter__.return_value = mock_uow
|
||||
MockUoW.return_value.__exit__.return_value = False
|
||||
|
||||
# Act
|
||||
_call_handler(
|
||||
{"property_ids": [pid], "portfolio_id": PORTFOLIO_ID,
|
||||
"scenario_id": SCENARIO_ID, "refetch_solar": False, "dry_run": False}
|
||||
)
|
||||
|
||||
# Assert — the other scenario's re-run does not become the default.
|
||||
plan_requests = mock_uow.plan.save_batch.call_args.args[0]
|
||||
assert plan_requests[0].is_default is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Lodged EPC path
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -233,7 +233,9 @@ class FakePlanRepository(PlanRepository):
|
|||
|
||||
def __init__(self) -> None:
|
||||
self.saved: dict[tuple[int, int], Plan] = {}
|
||||
self.default_property_ids: set[int] = set()
|
||||
# One default Plan per property: the scenario that currently owns it (a
|
||||
# new default demotes any prior one, mirroring the postgres invariant).
|
||||
self.default_scenario_by_property: dict[int, int] = {}
|
||||
self._next_id = 1
|
||||
|
||||
def save(
|
||||
|
|
@ -247,7 +249,7 @@ class FakePlanRepository(PlanRepository):
|
|||
) -> int:
|
||||
self.saved[(property_id, scenario_id)] = plan
|
||||
if is_default:
|
||||
self.default_property_ids.add(property_id)
|
||||
self.default_scenario_by_property[property_id] = scenario_id
|
||||
plan_id = self._next_id
|
||||
self._next_id += 1
|
||||
return plan_id
|
||||
|
|
@ -265,7 +267,18 @@ class FakePlanRepository(PlanRepository):
|
|||
]
|
||||
|
||||
def property_ids_with_default_plans(self, property_ids: list[int]) -> set[int]:
|
||||
return self.default_property_ids.intersection(property_ids)
|
||||
return {
|
||||
pid for pid in property_ids if pid in self.default_scenario_by_property
|
||||
}
|
||||
|
||||
def default_plan_scenario_by_property(
|
||||
self, property_ids: list[int]
|
||||
) -> dict[int, int]:
|
||||
return {
|
||||
pid: self.default_scenario_by_property[pid]
|
||||
for pid in property_ids
|
||||
if pid in self.default_scenario_by_property
|
||||
}
|
||||
|
||||
|
||||
class _UnsetProductRepo(ProductRepository):
|
||||
|
|
|
|||
|
|
@ -229,6 +229,29 @@ def test_reports_which_properties_already_have_a_default_plan(
|
|||
assert have_default == {20}
|
||||
|
||||
|
||||
def test_reports_the_scenario_holding_each_propertys_default_plan(
|
||||
db_engine: Engine,
|
||||
) -> None:
|
||||
# Arrange — property 40's default is scenario 7 (it also has a non-default
|
||||
# plan under scenario 8); property 41 has only a non-default plan; 42 none.
|
||||
with Session(db_engine) as session:
|
||||
repo = PlanPostgresRepository(session)
|
||||
repo.save(_plan(), property_id=40, scenario_id=7, portfolio_id=1, is_default=True)
|
||||
repo.save(_plan(), property_id=40, scenario_id=8, portfolio_id=1, is_default=False)
|
||||
repo.save(_plan(), property_id=41, scenario_id=7, portfolio_id=1, is_default=False)
|
||||
session.commit()
|
||||
|
||||
# Act
|
||||
with Session(db_engine) as session:
|
||||
by_property = PlanPostgresRepository(session).default_plan_scenario_by_property(
|
||||
[40, 41, 42]
|
||||
)
|
||||
|
||||
# Assert — only the property with a default Plan is present, mapped to the
|
||||
# scenario that owns that default (used to refresh the default in place).
|
||||
assert by_property == {40: 7}
|
||||
|
||||
|
||||
def test_a_new_default_demotes_the_prior_default_across_scenarios(
|
||||
db_engine: Engine,
|
||||
) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue