32 and delete in plan

repositories/plan/plan_postgres_repository.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from sqlmodel import Session, col, delete
+from sqlmodel import Session, col, update
 
 from domain.modelling.plan import Plan
 from infrastructure.postgres.modelling import PlanModel, RecommendationModel
@@ -10,7 +10,12 @@ from repositories.plan.plan_repository import PlanRepository
 class PlanPostgresRepository(PlanRepository):
     """Maps a Plan and its Plan Measures onto the live ``plan`` /
     ``recommendation`` tables (ADR-0017). Does not commit — the Unit of Work
-    owns the transaction (ADR-0012)."""
+    owns the transaction (ADR-0012).
+
+    A re-run INSERTs a fresh Plan rather than deleting the prior one (the cascade
+    delete was slow); when the new Plan is the default it demotes any prior
+    default Plan for the same (property_id, scenario_id) to ``is_default=False``,
+    so readers can select the current Plan via ``is_default=True``."""
 
     def __init__(self, session: Session) -> None:
         self._session = session
@@ -24,15 +29,20 @@ class PlanPostgresRepository(PlanRepository):
         portfolio_id: int,
         is_default: bool,
     ) -> int:
-        # Idempotent replace for (property_id, scenario_id): deleting the Plan
-        # cascades to its recommendation rows via the plan_id FK (ON DELETE
-        # CASCADE), so a re-run overwrites rather than duplicating (ADR-0012).
-        self._session.exec(  # type: ignore[call-overload]
-            delete(PlanModel).where(
-                col(PlanModel.property_id) == property_id,
-                col(PlanModel.scenario_id) == scenario_id,
+        # Soft-replace (ADR-0012): keep prior Plans as history rather than DELETEing
+        # them — the cascade delete of recommendation rows was the slow part. When
+        # this Plan is the default, demote every prior Plan for the same
+        # (property_id, scenario_id) to is_default=False, so exactly one Plan for
+        # the pair stays default (the one just inserted).
+        if is_default:
+            self._session.exec(  # type: ignore[call-overload]
+                update(PlanModel)
+                .where(
+                    col(PlanModel.property_id) == property_id,
+                    col(PlanModel.scenario_id) == scenario_id,
+                )
+                .values(is_default=False)
             )
-        )
 
         plan_row = PlanModel.from_domain(
             plan,

repositories/plan/plan_repository.py

@@ -8,10 +8,12 @@ from domain.modelling.plan import Plan
 class PlanRepository(ABC):
     """Persists a Plan (and its Plan Measures) for a Property + Scenario.
 
-    One Plan per (Property, Scenario). The write is idempotent on re-run: it
-    replaces the existing Plan for that pair rather than duplicating (ADR-0012
-    / ADR-0017). `portfolio_id` and `is_default` are supplied by the
-    orchestrator (the former from the trigger, the latter from the Scenario).
+    A re-run INSERTs a fresh Plan and keeps the prior one as history rather than
+    deleting it. When the new Plan is the default, prior default Plans for the
+    same (Property, Scenario) are demoted to `is_default=False`, so the current
+    Plan is the one with `is_default=True` (ADR-0012 / ADR-0017). `portfolio_id`
+    and `is_default` are supplied by the orchestrator (the former from the
+    trigger, the latter from the Scenario).
     """
 
     @abstractmethod
@@ -24,6 +26,7 @@ class PlanRepository(ABC):
         portfolio_id: int,
         is_default: bool,
     ) -> int:
-        """Persist ``plan`` and return its Plan id, replacing any existing Plan
-        for ``(property_id, scenario_id)``."""
+        """Persist ``plan`` and return its Plan id. Keeps prior Plans for
+        ``(property_id, scenario_id)`` as history; when ``is_default`` is True,
+        demotes those prior Plans to ``is_default=False``."""
         ...

tests/repositories/plan/test_plan_postgres_repository.py

@@ -2,10 +2,11 @@
 Plan Measures to the live ``plan`` / ``recommendation`` tables (ADR-0017).
 
 The Plan is the parent; each selected Plan Measure is a ``recommendation`` row
-linked by the new ``plan_id`` FK. A re-run replaces (delete the Plan for the
-(property, scenario) → cascade its recommendations → insert fresh), so the
-batch write is idempotent (ADR-0012). CO₂ is stored in tonnes (calculator kg
-÷ 1000) to match the live column contract.
+linked by the new ``plan_id`` FK. A re-run INSERTs a fresh Plan and keeps the
+prior one as history (no cascade delete); when the new Plan is the default it
+demotes prior default Plans for the (property, scenario) to ``is_default=False``
+(ADR-0012). CO₂ is stored in tonnes (calculator kg ÷ 1000) to match the live
+column contract.
 """
 
 from __future__ import annotations
@@ -147,31 +148,61 @@ def test_save_persists_null_per_measure_savings_when_unbilled(
     assert rec_rows[0].energy_cost_savings is None
 
 
-def test_save_is_idempotent_on_rerun_for_the_same_property_and_scenario(
+def test_rerun_keeps_history_and_demotes_the_prior_default_plan(
     db_engine: Engine,
 ) -> None:
-    # Arrange — first run
+    # Arrange — first (default) run
     with Session(db_engine) as session:
-        PlanPostgresRepository(session).save(
+        first_id = PlanPostgresRepository(session).save(
             _plan(), property_id=10, scenario_id=7, portfolio_id=1, is_default=True
         )
         session.commit()
 
-    # Act — re-run the same (property, scenario)
+    # Act — re-run the same (property, scenario) as the default
     with Session(db_engine) as session:
-        PlanPostgresRepository(session).save(
+        second_id = PlanPostgresRepository(session).save(
             _plan(), property_id=10, scenario_id=7, portfolio_id=1, is_default=True
         )
         session.commit()
 
-    # Assert — replaced, not duplicated (cascade removed the old measures)
+    # Assert — the prior Plan is kept as history (no delete), and only the new
+    # Plan is the default; exactly one Plan for the pair stays is_default=True.
     with Session(db_engine) as session:
         plan_rows = session.exec(
             select(PlanModel).where(col(PlanModel.property_id) == 10)
         ).all()
-        rec_rows = session.exec(
-            select(RecommendationModel).where(col(RecommendationModel.property_id) == 10)
-        ).all()
+        by_id = {p.id: p for p in plan_rows}
 
-    assert len(plan_rows) == 1
-    assert len(rec_rows) == 1
+    assert len(plan_rows) == 2
+    assert first_id != second_id
+    assert by_id[first_id].is_default is False  # demoted
+    assert by_id[second_id].is_default is True  # the current default
+    assert sum(1 for p in plan_rows if p.is_default) == 1
+
+
+def test_rerun_as_non_default_does_not_demote_the_prior_default(
+    db_engine: Engine,
+) -> None:
+    # Arrange — a default Plan exists
+    with Session(db_engine) as session:
+        first_id = PlanPostgresRepository(session).save(
+            _plan(), property_id=12, scenario_id=7, portfolio_id=1, is_default=True
+        )
+        session.commit()
+
+    # Act — re-run as NON-default (e.g. a non-default scenario); no demotion runs
+    with Session(db_engine) as session:
+        PlanPostgresRepository(session).save(
+            _plan(), property_id=12, scenario_id=7, portfolio_id=1, is_default=False
+        )
+        session.commit()
+
+    # Assert — the prior default is untouched (we only demote when saving a default)
+    with Session(db_engine) as session:
+        plan_rows = session.exec(
+            select(PlanModel).where(col(PlanModel.property_id) == 12)
+        ).all()
+        by_id = {p.id: p for p in plan_rows}
+
+    assert len(plan_rows) == 2
+    assert by_id[first_id].is_default is True