Merge branch 'main' of https://github.com/Hestia-Homes/Model into bug/plan-with-budget-more-expensive

backend/Property.py

@@ -490,7 +490,7 @@ class Property:
         for rec_id in rec_ids:
             sim_epc = self.simulation_epcs[rec_id].copy()
             rec_impact = [x for x in impact_summary if x["recommendation_id"] == rec_id][0]
-            # We update all of the features that should have an impact on the kwh model
+            # We update all features that should have an impact on the kwh model
 
             sim_epc.update(
                 {

backend/app/db/functions/recommendations_functions.py

@@ -1,5 +1,5 @@
-from typing import Any, Dict, List, Optional
-from sqlalchemy import inspect, text, insert, delete, select, update
+from typing import Any, Dict, List, Tuple
+from sqlalchemy import inspect, text, insert, delete, select
 from sqlalchemy.orm import Session, Mapper
 from sqlalchemy.exc import SQLAlchemyError
 from sqlmodel import Session
@@ -618,18 +618,72 @@ def clear_portfolio_in_batches(
     print("Portfolio cleared in batches.")
 
 
-def get_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]:
-    stmt = select(PlanModel).where(PlanModel.portfolio_id == portfolio_id)
+def get_plans_by_scenario_ids(ids: List[int]) -> List[PlanModel]:
+    stmt = select(PlanModel).where(PlanModel.scenario_id.in_(ids))
     with db_read_session() as session:
         session_any: Any = session  # Typehint as Any to satisfy Pylance...
         return session_any.exec(stmt).scalars().all()
 
 
-def get_scenario(scenario_id: int) -> Optional[ScenarioModel]:
-    stmt = select(ScenarioModel).where(ScenarioModel.id == scenario_id)
+def get_most_recent_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]:
+    # NOTE: This statement works for Postgres only, because of the Distinct
+    stmt = (
+        select(PlanModel)
+        .where(PlanModel.portfolio_id == portfolio_id)
+        .distinct(
+            PlanModel.property_id, PlanModel.scenario_id
+        )  # one plan per property per scenario
+        .order_by(
+            PlanModel.property_id,
+            PlanModel.scenario_id,
+            PlanModel.created_at.desc(),
+            PlanModel.id.desc(),
+        )
+    )
     with db_read_session() as session:
         session_any: Any = session  # Typehint as Any to satisfy Pylance...
-        return session_any.exec(stmt).scalar_one_or_none()
+        return session_any.exec(stmt).scalars().all()
+
+
+def get_most_recent_plans_by_scenario_ids(scenario_ids: List[int]) -> List[PlanModel]:
+    # NOTE: This statement works for Postgres only, because of the Distinct
+    stmt = (
+        select(PlanModel)
+        .where(PlanModel.scenario_id.in_(scenario_ids))
+        .distinct(
+            PlanModel.property_id, PlanModel.scenario_id
+        )  # one plan per property per scenario
+        .order_by(
+            PlanModel.property_id,
+            PlanModel.scenario_id,
+            PlanModel.created_at.desc(),
+            PlanModel.id.desc(),
+        )
+    )
+
+    with db_read_session() as session:
+        session_any: Any = session  # Typehint as Any to satisfy Pylance
+        return session_any.exec(stmt).scalars().all()
+
+
+def get_scenarios_by_portfolio_id(portfolio_id: int) -> List[ScenarioModel]:
+    stmt = select(ScenarioModel).where(ScenarioModel.portfolio_id == portfolio_id)
+    with db_read_session() as session:
+        session_any: Any = session  # Typehint as Any to satisfy Pylance...
+        return session_any.exec(stmt).scalars().all()
+
+
+def get_default_plans(
+    portfolio_id: int,
+) -> List[PlanModel]:
+    plan_stmt = select(PlanModel).where(
+        (PlanModel.portfolio_id == portfolio_id) & (PlanModel.is_default == True)
+    )
+
+    with db_read_session() as session:
+        session_any: Any = session  # Typehint as Any to satisfy Pylance...
+        plans: List[PlanModel] = session_any.exec(plan_stmt).scalars().all()
+        return plans
 
 
 def bulk_update_plans(

backend/app/domain/classes/plan.py

@@ -47,6 +47,7 @@ class Plan:
             valuation_increase=plan_model.valuation_increase,
             cost_of_works=plan_model.cost_of_works,
             contingency_cost=plan_model.contingency_cost,
+            name=plan_model.name,
         )
         return cls(record=record, scenario=scenario, id=plan_model.id)
 
@@ -60,6 +61,14 @@ class Plan:
             case _:
                 raise NotImplementedError
 
+    @property
+    def cost(self) -> float:
+        return (
+            self.record.cost_of_works
+            if self.record.cost_of_works is not None
+            else float("inf")
+        )
+
     def to_sqlalchemy(self) -> PlanPersistence:
         scenario_record = self.scenario.record
 
@@ -129,6 +138,7 @@ class Plan:
             valuation_increase=record.valuation_increase,
             cost_of_works=record.cost_of_works,
             contingency_cost=record.contingency_cost,
+            name=record.name,
         )
 
         return PlanPersistence(plan=plan_model, scenario=scenario_model)

backend/app/domain/records/plan_record.py

@@ -29,3 +29,4 @@ class PlanRecord:
     valuation_increase: Optional[float] = None
     cost_of_works: Optional[float] = None
     contingency_cost: Optional[float] = None
+    name: Optional[str] = None

backend/categorisation/categorisation_trigger_request.py

@@ -1,5 +1,12 @@
+from typing import List, Optional
 from pydantic import BaseModel
 
 
 class CategorisationTriggerRequest(BaseModel):
     portfolio_id: int
+
+    scenarios_to_consider: Optional[List[int]] = None
+    scenario_priority_order: Optional[List[int]] = None
+
+
+# {"portfolio_id": 556, "scenarios_to_consider": [1039,1041], "scenario_priority_order": [1041,1039]}

backend/categorisation/handler/handler.py

@@ -1,5 +1,6 @@
 import json
 from typing import Any, Mapping
+
 from backend.categorisation.categorisation_trigger_request import (
     CategorisationTriggerRequest,
 )
@@ -12,6 +13,10 @@ logger = setup_logger()
 
 def handler(event: Mapping[str, Any], context: Any) -> None:
 
+    logger.info("Received message")
+
+    logger.info(f"Number of events: {len(event.get('Records', []))}")
+
     for record in event.get("Records", []):
         try:
             body_dict = json.loads(record["body"])
@@ -20,7 +25,12 @@ def handler(event: Mapping[str, Any], context: Any) -> None:
 
             logger.debug("Successfully validated request body")
 
-            process_portfolio(payload.portfolio_id)
+            process_portfolio(
+                payload.portfolio_id,
+                payload.scenarios_to_consider,
+                payload.scenario_priority_order,
+            )
 
         except Exception as e:
+            logger.info("Handler exception")
             logger.error(f"Failed to process record: {e}")

backend/categorisation/local_handler/docker-compose.yml

@@ -0,0 +1,11 @@
+version: "3.9"
+
+services:
+  categorisation-lambda:
+    build:
+      context: ../../../
+      dockerfile: backend/categorisation/handler/Dockerfile
+    ports:
+      - "9000:8080"
+    env_file:
+      - ../../../.env

backend/categorisation/local_handler/invoke_local_lambda.py

@@ -0,0 +1,25 @@
+#!/usr/bin/env python3
+import json
+import requests
+
+LAMBDA_URL = "http://localhost:9000/2015-03-31/functions/function/invocations"
+
+payload = {
+    "Records": [
+        {
+            "body": json.dumps(
+                {
+                    "portfolio_id": 556,
+                    "scenarios_to_consider": [],
+                    "scenario_priority_order": [],
+                }
+            )
+        }
+    ]
+}
+
+response = requests.post(LAMBDA_URL, json=payload)
+
+print("Status code:", response.status_code)
+print("Response:")
+print(response.text)

backend/categorisation/local_runner.py

@@ -1,10 +1,18 @@
+from typing import List
+
 from backend.categorisation.processor import process_portfolio
 
 
 def main() -> None:
     portfolio_id = 556
+    scenarios_to_consider: List[int] = []
+    scenario_priority_order: List[int] = []
 
-    process_portfolio(portfolio_id)
+    process_portfolio(
+        portfolio_id=portfolio_id,
+        scenarios_to_consider=scenarios_to_consider,
+        scenario_priority_order=scenario_priority_order,
+    )
 
 
 if __name__ == "__main__":

backend/categorisation/processor.py

@@ -1,10 +1,12 @@
 from collections import defaultdict
-from typing import Dict, List
+from typing import Dict, List, Optional
 
 from backend.app.db.functions.recommendations_functions import (
     bulk_update_plans,
-    get_plans_by_portfolio_id,
-    get_scenario,
+    get_default_plans,
+    get_most_recent_plans_by_portfolio_id,
+    get_most_recent_plans_by_scenario_ids,
+    get_scenarios_by_portfolio_id,
 )
 from backend.app.db.models.recommendations import PlanModel, ScenarioModel
 from backend.app.domain.classes.plan import Plan
@@ -14,37 +16,152 @@ from utils.logger import setup_logger
 logger = setup_logger()
 
 
-def process_portfolio(portfolio_id: int) -> None:
-    print(f"Processing portfolio {portfolio_id}")
-    plans: List[Plan] = _load_plans_for_portfolio(portfolio_id)
-    plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans)
+def process_portfolio(
+    portfolio_id: int,
+    scenarios_to_consider: Optional[List[int]] = None,
+    scenario_priority_order: Optional[List[int]] = None,
+) -> None:  # TODO: make this a class
+    logger.info(f"Processing portfolio {portfolio_id}")
 
-    for uprn, property_plans in plans_by_property.items():
+    all_scenarios: List[Scenario] = _load_scenarios_for_portfolio(portfolio_id)
+    plans_by_id: Dict[int, Plan] = {}  # TODO: make this an in-memory repository class
 
+    if scenarios_to_consider:
+        if len(scenarios_to_consider) < 2:
+            raise ValueError(
+                "Cannot run auto categorisation for fewer than 2 scenarios"
+            )
+
+    # first get all plans that we're interested in
+    plans_for_consideration: List[Plan] = _load_plans_for_portfolio(
+        portfolio_id, all_scenarios, scenarios_to_consider
+    )
+    for plan in plans_for_consideration:
+        if plan.id is not None:  # just in case
+            plans_by_id[plan.id] = plan
+
+    # then unset existing defaults on domain objects regardless of whether they're under consideration or not
+    default_plans: List[Plan] = _get_default_plans(portfolio_id, all_scenarios)
+    for plan in default_plans:
+        plan.set_default(False)
+        if plan.id is not None:  # just in case
+            plans_by_id[plan.id] = plan
+
+    logger.info(f"Successfully unset {len(default_plans)} default plan(s)")
+
+    # then set new defaults on domain objects under consideration
+    plans_for_consideration_by_property: Dict[int, List[Plan]] = (
+        _group_plans_by_property(plans_for_consideration)
+    )
+
+    for property_id, property_plans in plans_for_consideration_by_property.items():
         if not property_plans:
-            raise ValueError(f"No plans for property {uprn}")
+            raise ValueError(f"No plans for property {property_id}")
 
-        cheapest_plan = _choose_cheapest_relevant_plan(property_plans)
-        _update_default_flags(property_plans, cheapest_plan)
+        try:
+            cheapest_plan = choose_cheapest_relevant_plan(
+                property_plans, scenario_priority_order
+            )
+        except Exception:
+            logger.error(f"Failed to find cheapest plan for property {property_id}")
+            raise
+
+        property_plans = _update_plan_objects(property_plans, cheapest_plan)
+        for plan in property_plans:
+            if plan.id is not None:  # just in case
+                plans_by_id[plan.id] = plan
+
+    logger.info("Successfully set defaults on Plan objects in memory")
+
+    # then pass all domain objects to database to update (regardless of whether they've changed)
+    _update_plans_in_db(list(plans_by_id.values()))
+    logger.info(f"Successfully updated {len(plans_by_id)} Plans in database")
 
 
-def _load_plans_for_portfolio(portfolio_id: int) -> List[Plan]:
-    plan_models = get_plans_by_portfolio_id(portfolio_id)
-    print(f"Got {len(plan_models)} plans from database")
+def choose_cheapest_relevant_plan(
+    plans: List[Plan], scenario_priority_order: Optional[List[int]] = None
+) -> Plan:
+    scenario_priority_order = scenario_priority_order or []
+
+    eligible_plans: List[Plan] = [plan for plan in plans if plan.is_compliant] or plans
+    if not eligible_plans:
+        raise ValueError("No plans available to choose from.")
+
+    for plan in eligible_plans:
+        if plan.id is None:
+            # This should never actually happen, but plan.id is optional to cater
+            # for new plans. We are only working with already persisted plans here
+            raise ValueError(
+                f"All plans must have an ID, but found a plan with no ID: {plan}"
+            )
+
+    min_cost: float = min(plan.cost for plan in eligible_plans)
+
+    cheapest_plans: List[Plan] = [
+        plan for plan in eligible_plans if plan.cost == min_cost
+    ]
+
+    for priority_scenario_id in scenario_priority_order:
+        for plan in cheapest_plans:
+            if plan.scenario.id == priority_scenario_id:
+                return plan
+
+    return cheapest_plans[0]
+
+
+def _get_default_plans(portfolio_id: int, scenarios: List[Scenario]) -> List[Plan]:
+    default_plan_models = get_default_plans(portfolio_id)
+
+    scenario_map = {s.id: s for s in scenarios}
+
+    return [
+        Plan.from_sqlalchemy(p, scenario_map[p.scenario_id])
+        for p in default_plan_models
+        if p.scenario_id in scenario_map
+    ]
+
+
+def _load_scenarios_for_portfolio(portfolio_id: int) -> List[Scenario]:
+    scenario_models: List[ScenarioModel] = get_scenarios_by_portfolio_id(portfolio_id)
+
+    return [Scenario.from_sqlalchemy(s) for s in scenario_models]
+
+
+def _load_plans_for_portfolio(
+    portfolio_id: int,
+    all_scenarios: List[Scenario],
+    scenarios_to_consider: Optional[List[int]] = None,
+) -> List[Plan]:
+
+    if scenarios_to_consider:
+        logger.info(f"Getting plans for {len(scenarios_to_consider)} scenarios")
+        plan_models: List[PlanModel] = get_most_recent_plans_by_scenario_ids(
+            scenarios_to_consider
+        )
+        logger.info(f"Got {len(plan_models)} plan models from database")
+    else:
+        logger.info(
+            f"No list of Plans to consider provided. Getting all Plans for portfolio {portfolio_id}"
+        )
+        plan_models: List[PlanModel] = get_most_recent_plans_by_portfolio_id(
+            portfolio_id
+        )
 
     plans: List[Plan] = []
 
+    if not all_scenarios:
+        raise Exception(f"No scenarios found for Portfolio {portfolio_id}")
+
     for model in plan_models:
-        if not model.scenario_id:
+
+        scenario = next((s for s in all_scenarios if s.id == model.scenario_id))
+        if not scenario:
             logger.info(f"No Scenario associated with Plan of ID {model.id}")
             continue
 
-        scenario_model = get_scenario(model.scenario_id)
-        plans.append(
-            Plan.from_sqlalchemy(model, Scenario.from_sqlalchemy(scenario_model))
-        )
-        print("Successfully mapped plan and scenario to domain object")
+        plans.append(Plan.from_sqlalchemy(model, scenario))
 
+    logger.info(f"Got {len(plans)} Plans")
     return plans
 
 
@@ -57,37 +174,26 @@ def _group_plans_by_property(plans: List[Plan]) -> Dict[int, List[Plan]]:
     return grouped
 
 
-def _choose_cheapest_relevant_plan(plans: List[Plan]) -> Plan:
-    plans_to_consider: List[Plan] = [p for p in plans if p.is_compliant] or plans
-
-    def plan_cost(plan: Plan) -> float:
-        return (
-            plan.record.cost_of_works
-            if plan.record.cost_of_works is not None
-            else float("inf")
-        )
-
-    cheapest_plan = min(plans_to_consider, key=plan_cost)
-
-    return cheapest_plan
-
-
-def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None:
-    plans_to_update: List[Plan] = []
-
+def _update_plan_objects(plans: List[Plan], cheapest_plan: Plan) -> List[Plan]:
     for plan in plans:
         should_be_default: bool = plan.id == cheapest_plan.id
-        if plan.record.is_default != should_be_default:
-            plan.set_default(should_be_default)
-            plans_to_update.append(plan)
+        plan.set_default(should_be_default)
 
-    if plans_to_update:
-        plan_models: List[PlanModel] = []
-        scenario_models: List[ScenarioModel] = []
+        if should_be_default:
+            logger.debug(
+                f"Setting Plan {plan.id} (Scenario Name: {plan.scenario.record.name}) to default"
+            )
 
-        for plan in plans_to_update:
-            plan_model, scenario_model = plan.to_sqlalchemy()
-            plan_models.append(plan_model)
-            scenario_models.append(scenario_model)
+    return plans
 
-        bulk_update_plans(plan_models, scenario_models)
+
+def _update_plans_in_db(plans: List[Plan]) -> None:
+    plan_models: List[PlanModel] = []
+    scenario_models: List[ScenarioModel] = []
+
+    for plan in plans:
+        plan_model, scenario_model = plan.to_sqlalchemy()
+        plan_models.append(plan_model)
+        scenario_models.append(scenario_model)
+
+    bulk_update_plans(plan_models, scenario_models)

backend/categorisation/tests/test_prioritised_plan_selected.py

@@ -0,0 +1,160 @@
+from datetime import datetime
+from typing import List, Optional
+import pytest
+
+from backend.app.domain.classes.plan import Plan
+from backend.app.domain.classes.scenario import Scenario
+from backend.app.domain.records.plan_record import PlanRecord
+from backend.app.domain.records.scenario_record import ScenarioRecord
+from backend.app.db.models.portfolio import Epc, PortfolioGoal
+from backend.categorisation.processor import choose_cheapest_relevant_plan
+
+
+@pytest.fixture
+def created_at_datetime() -> datetime:
+    return datetime.now()
+
+
+def make_plan_record(
+    created_at: datetime, default: bool, cost_of_works: Optional[float] = 500.0
+) -> PlanRecord:
+    return PlanRecord(
+        property_id=1,
+        portfolio_id=1,
+        created_at=created_at,
+        is_default=default,
+        post_epc_rating=Epc.C,
+        cost_of_works=cost_of_works,
+    )
+
+
+def make_scenario(name: str, created_at: datetime, is_default: bool) -> Scenario:
+    record = ScenarioRecord(
+        name=name,
+        created_at=created_at,
+        housing_type="",
+        goal=PortfolioGoal.INCREASING_EPC,
+        goal_value="C",
+        trigger_file_path="",
+        multi_plan=False,
+        is_default=is_default,
+    )
+    return Scenario(record=record, id=3 if is_default else 4)
+
+
+def make_plan(
+    created_at: datetime,
+    default: bool,
+    cost_of_works: Optional[float] = 500.0,
+    name: str = "",
+) -> Plan:
+    scenario = make_scenario(name, created_at, default)
+    plan_id = 1 if default else 2
+    return Plan(
+        record=make_plan_record(created_at, default, cost_of_works),
+        scenario=scenario,
+        id=plan_id,
+    )
+
+
+def test_prioritised_scenario_selected(created_at_datetime: datetime) -> None:
+    # arrange
+    epc_c_plan = make_plan(created_at_datetime, True, name="EPC C")
+    minor_works_plan = make_plan(created_at_datetime, False, name="EPC C - Minor Works")
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 2
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id
+
+
+def test_cheapest_plan_returned_if_not_in_priority_list(
+    created_at_datetime: datetime,
+) -> None:
+    # arrange
+    epc_c_plan = make_plan(
+        created_at_datetime, True, cost_of_works=1000.0, name="EPC C"
+    )
+    minor_works_plan = make_plan(
+        created_at_datetime, False, cost_of_works=100.0, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [3, 5]
+    expected_default_plan_id = 2
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id
+
+
+def test_all_plans_zero_cost__highest_priority_returned(
+    created_at_datetime: datetime,
+) -> None:
+    # arrange
+    epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=0.0, name="EPC C")
+    minor_works_plan = make_plan(
+        created_at_datetime, False, cost_of_works=0.0, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 2
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id
+
+
+def test_some_plans_zero_cost__cheapest_returned(
+    created_at_datetime: datetime,
+) -> None:
+    # arrange
+    epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=0.0, name="EPC C")
+    minor_works_plan = make_plan(
+        created_at_datetime, False, cost_of_works=50.0, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 1
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id
+
+
+def test_all_plans_null_cost__highest_priority_returned(
+    created_at_datetime: datetime,
+) -> None:
+    # arrange
+    epc_c_plan = make_plan(created_at_datetime, True, cost_of_works=None, name="EPC C")
+    minor_works_plan = make_plan(
+        created_at_datetime, False, cost_of_works=None, name="EPC C - Minor Works"
+    )
+    scenario_priority_order: List[int] = [4, 3]
+    expected_default_plan_id = 2
+
+    # act
+    actual_default_plan = choose_cheapest_relevant_plan(
+        plans=[epc_c_plan, minor_works_plan],
+        scenario_priority_order=scenario_priority_order,
+    )
+
+    # assert
+    assert actual_default_plan.id == expected_default_plan_id

backend/docker-compose-local-lambdas.yml

@@ -0,0 +1,11 @@
+version: "3.9"
+
+services:
+  categorisation-lambda:
+    build:
+      context: ../
+      dockerfile: backend/categorisation/handler/Dockerfile
+    ports:
+      - "9000:8080"
+    env_file:
+      - ../.env

backend/engine/engine.py

@@ -1053,14 +1053,18 @@ async def model_engine(body: PlanTriggerRequest):
             property_required_measures = [m for m in recommendations[p.id] if m[0]["type"] in body.required_measures]
             measures_to_optimise = [m for m in recommendations[p.id] if m[0]["type"] not in body.required_measures]
 
-            ventilation_included = "ventilation" in property_measure_types
+            # TODO - formalise property measure types into an enum
+            ventilation_included = (
+                "ventilation" in property_measure_types or "mechanical_ventilation" in property_measure_types
+            )
 
             # If a measure requiring ventilation is selected, and the property does not have ventilation, we enfore
             # its inclusion
 
-            needs_ventilation = any(
-                x in property_measure_types for x in assumptions.measures_needing_ventilation
-            ) and not p.has_ventilation and ventilation_included
+            needs_ventilation = optimiser_functions.check_needs_ventilation(
+                property_measure_types, assumptions.measures_needing_ventilation, p.has_ventilation,
+                ventilation_included
+            )
 
             if not measures_to_optimise:
                 # Nothing to do, we just reshape the recommendations
@@ -1177,7 +1181,7 @@ async def model_engine(body: PlanTriggerRequest):
                     recommendations=recommendations, selected=selected,
                 )
 
-            # Add best practice measures (ventilation/trickle vents)
+            # Add best practice measures (ventilation/trickle vents) - pass needs_ventilation flag
             selected = optimiser_functions.add_best_practice_measures(p.id, solution, recommendations, selected)
             # Final flattening - we pass what the battery SAP score would be, regardless if the battery was selected
             recommendations[p.id] = optimiser_functions.flatten_recommendations_with_defaults(

recommendations/Recommendations.py

@@ -499,8 +499,16 @@ class Recommendations:
         return predicted_appliances_cost_reduction, predicted_appliances_kwh_reduction
 
     @staticmethod
-    def _check_ventilation_out_of_bounds(sap_impact, ventilation_sap_limit):
-        return (sap_impact < ventilation_sap_limit) or (sap_impact >= 0)
+    def _check_ventilation_out_of_bounds(sap_impact: float, ventilation_sap_limit: float) -> bool:
+        """
+        Checks if the SAP impact of a ventilation recommendation is out of bounds, which would indicate that the
+        recommendation is not appropriate.
+        :param sap_impact: The SAP impact of the ventilation recommendation, which is typically negative or zero
+        :param ventilation_sap_limit: The SAP limit for ventilation recommendations, which is typically a negative
+        number. E.g. -4
+        :return:
+        """
+        return (sap_impact < ventilation_sap_limit) or (sap_impact > 0)
 
     @staticmethod
     def _adjust_ventilation_sap(sap_impact, ventilation_sap_limit):
@@ -574,6 +582,7 @@ class Recommendations:
         if rec_phase == starting_phase:
             return {
                 "sap": float(property_instance.data["current-energy-efficiency"]),
+                "sap_prediction": float(property_instance.data["current-energy-efficiency"]),
                 "carbon": float(property_instance.data["co2-emissions-current"]),
                 "heat_demand": float(property_instance.data["energy-consumption-current"]),
             }
@@ -591,12 +600,13 @@ class Recommendations:
         if not previous_phase_reps:
             return {
                 "sap": float(property_instance.data["current-energy-efficiency"]),
+                "sap_prediction": float(property_instance.data["current-energy-efficiency"]),
                 "carbon": float(property_instance.data["co2-emissions-current"]),
                 "heat_demand": float(property_instance.data["energy-consumption-current"]),
             }
 
         # Median fallback (including zero-length case)
-        keys = ("sap", "carbon", "heat_demand")
+        keys = ("sap", "sap_prediction", "carbon", "heat_demand")
         return {
             key: np.median([item[key] for item in previous_phase_reps])
             for key in keys
@@ -691,7 +701,8 @@ class Recommendations:
         previous_phase_values: dict,
         current_phase_values: dict,
         adjustments: list,
-        property_instance,
+        property_instance: Property,
+        model_predicted_sap: float,
     ):
         # For the moment, we cap the number of SAP points that can be achieved by LEDs at 2
         if rec["type"] == "low_energy_lighting":
@@ -785,7 +796,6 @@ class Recommendations:
 
                 # Update the current phase values
                 current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"]
-
         elif rec["type"] == "loft_insulation":
             # When we have a loft insulation recommendation, where there is an extension and the existing
             # amount of loft insulation is already good, we limit the SAP points
@@ -831,6 +841,27 @@ class Recommendations:
 
             # Update the current phase values
             current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"]
+        elif rec["measure_type"] in ["roomstat_programmer_trvs", "time_temperature_zone_control"]:
+            # We trim the SAP point recommendations based on the minimum of the predicted and the survey SAP
+            # points
+            predicted_difference = model_predicted_sap - previous_phase_values["sap_prediction"]
+            proposed_impact = property_phase_impact["sap"]
+            numerically_the_same = np.isclose(proposed_impact, predicted_difference)
+
+            if predicted_difference > 0 and (predicted_difference < proposed_impact) and not numerically_the_same:
+                # We constrain the impact based on what the model predicts.
+                # We update the proposed impact to be the predicted difference
+                adjustments.append(
+                    {
+                        "recommendation_id": rec["recommendation_id"],
+                        "phase": rec["phase"],
+                        # If we've made an adjustment, it will be negative
+                        "sap_adjustment": property_phase_impact["sap"] - predicted_difference,
+                    }
+                )
+                property_phase_impact["sap"] = predicted_difference
+                # Update the current phase values
+                current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"]
 
         return property_phase_impact, current_phase_values, adjustments
 
@@ -963,7 +994,8 @@ class Recommendations:
                     previous_phase_values=previous_phase_values,
                     current_phase_values=current_phase_values,
                     adjustments=adjustments,
-                    property_instance=property_instance
+                    property_instance=property_instance,
+                    model_predicted_sap=phase_energy_efficiency_metrics["sap_change"],
                 )
 
                 # Insert this information into the recommendation.

recommendations/SecondaryHeating.py

@@ -18,6 +18,9 @@ class SecondaryHeating:
     def recommend(self, phase: int):
         # Reset
         self.recommendation = []
+        if self.property.epc_record.secondheat_description in ["None", None]:
+            # No secondary heating system, so no recommendation to remove it
+            return
 
         if self.property.data['number-habitable-rooms'] > self.property.data['number-heated-rooms']:
             n_rooms = self.property.data['number-habitable-rooms'] - self.property.data['number-heated-rooms']

recommendations/optimiser/funding_optimiser.py

@@ -655,6 +655,11 @@ def optimise_with_scenarios(
     1) With air source heat pump AND required insulation
     """
 
+    # Universally handle zero gain
+    if target_gain is not None:
+        if target_gain <= 0:
+            return pd.DataFrame([])
+
     solutions = []
     paths = []
     # Produce the unique list of measure types
@@ -770,6 +775,11 @@ def optimise_with_scenarios(
 
     for fixed in fixed_selections:
 
+        if target_gain is not None:
+            if target_gain <= 0:
+                # If we don't have any gain, we don't actually need to do this
+                continue
+
         # fixed = [(gi, oi, opt), ...]
         fixed_items = [opt for (_, _, opt) in fixed]
         fixed_groups = {gi for (gi, _, _) in fixed}

recommendations/optimiser/optimiser_functions.py

@@ -1,4 +1,5 @@
 import pandas as pd
+from typing import List, Dict, Any, Set
 import backend.app.assumptions as assumptions
 from backend.Property import Property
 from backend.app.plan.schemas import PlanTriggerRequest
@@ -78,14 +79,14 @@ def prepare_input_measures(
         # if recs[0]["type"] == "solar_pv":
         #     recs = [r for r in recs if ~r["has_battery"]]
 
-        # Only include measures with non-negative cost savings
+        # Only include measures with non-negative cost savings - we allow for a minor negative impact
         if eco_measures:
             recs_to_append = [
-                rec for rec in recs if (rec["energy_cost_savings"] >= 0) or (rec["measure_type"] in eco_measures)
+                rec for rec in recs if (rec["energy_cost_savings"] >= -10) or (rec["measure_type"] in eco_measures)
             ]
         else:
             recs_to_append = [
-                rec for rec in recs if (rec["energy_cost_savings"] >= 0)
+                rec for rec in recs if (rec["energy_cost_savings"] >= -10)
             ]
         if not recs_to_append:
             continue
@@ -300,7 +301,12 @@ def add_required_measures(property_id, property_required_measures, recommendatio
     ]
 
 
-def add_best_practice_measures(property_id, solution, recommendations, selected):
+def add_best_practice_measures(
+    property_id: int,
+    solution: List[Dict[str, Any]],
+    recommendations: Dict[int, List[List[Dict[str, Any]]]],
+    selected: Set[str],
+):
     """
     Ensures best-practice measures like ventilation and trickle vents are included
     in the selected recommendations when appropriate.
@@ -331,11 +337,11 @@ def add_best_practice_measures(property_id, solution, recommendations, selected)
 
     # If ventilation has been selected, or one of the measures needs ventilation, we need to ensure ventilation is
     # included
-    needs_ventilation = any(
+    measures_selected_needing_ventilation = any(
         x in [r["type"] for r in solution] for x in assumptions.measures_needing_ventilation
-    ) or len(ventilation_selected) > 0
+    )
 
-    if needs_ventilation:
+    if measures_selected_needing_ventilation or len(ventilation_selected) > 0:
         ventilation_rec = next(
             (r[0] for r in recommendations[property_id] if r[0]["type"] == "mechanical_ventilation"),
             None
@@ -395,3 +401,30 @@ def flatten_recommendations_with_defaults(property_id, recommendations, selected
 
     # Flatten the nested list of lists into a single list
     return [rec for recommendations_by_type in final_recommendations for rec in recommendations_by_type]
+
+
+def check_needs_ventilation(
+    property_measure_types: Set[str],
+    measures_needing_ventilation: List[str],
+    property_already_has_ventilation: bool,
+    ventilation_in_included_measures: bool
+) -> bool:
+    """
+    Function to check if we need to include ventilation based on the measures selected and the property
+    features
+    :param property_measure_types: The set of measure types recommended for the property
+    :param measures_needing_ventilation: The set of measure types that require ventilation
+    :param property_already_has_ventilation: Whether the property currently has ventilation
+    :param ventilation_in_included_measures: Whether ventilation is already included in the recommended
+    measures
+    :return: Boolean indicating whether ventilation needs to be included in the recommendations
+
+    # TODO - none of the inputs of this function are well structured and so this is quite brittle - we should
+             consider refactoring to make this more robust
+    """
+
+    needs_ventilation = any(
+        x in property_measure_types for x in measures_needing_ventilation
+    )
+
+    return needs_ventilation and not property_already_has_ventilation and ventilation_in_included_measures

recommendations/tests/test_optimiser_functions.py

@@ -3,9 +3,19 @@ import numpy as np
 from types import SimpleNamespace
 from recommendations.tests.test_data.measures_to_optimise import measures_to_optimise
 from recommendations.optimiser import optimiser_functions
+from recommendations.optimiser.funding_optimiser import optimise_with_scenarios
 from recommendations.optimiser.GainOptimiser import GainOptimiser
 from recommendations.optimiser.CostOptimiser import CostOptimiser
-from recommendations.optimiser.StrategicOptimiser import StrategicOptimiser, Strategies
+from recommendations.optimiser.StrategicOptimiser import StrategicOptimiser
+
+
+@pytest.fixture
+def property_instance():
+    return SimpleNamespace(
+        id="P1",
+        has_ventilation=False,
+        data={"current-energy-efficiency": "52"},
+    )
 
 
 class TestPrepareInputMeasures:
@@ -48,8 +58,9 @@ class TestPrepareInputMeasures:
     def test_filters_out_negative_cost_savings(self):
         recs = [
             [{"recommendation_id": "bad1", "type": "loft_insulation", "total": 200, "kwh_savings": 100,
-              "energy_cost_savings": -5, "has_battery": False,
-              "partial_project_funding": 0, "partial_project_score": 0, "uplift_project_score": 0, }],
+              "energy_cost_savings": -100, "has_battery": False,
+              "partial_project_funding": 0, "partial_project_score": 0, "uplift_project_score": 0,
+              "measure_type": "roof_insulation"}],
         ]
         measures = optimiser_functions.prepare_input_measures(recs, goal="Energy Savings", needs_ventilation=False)
         assert measures == []  # should skip negative cost saving recs
@@ -143,7 +154,9 @@ class TestAddBestPracticeMeasures:
             ]
         }
         selected = set()
-        updated = optimiser_functions.add_best_practice_measures(property_id, solution, recommendations, selected)
+        updated = optimiser_functions.add_best_practice_measures(
+            property_id, solution, recommendations, selected
+        )
         assert "vent1" in updated
         assert "trickle1" in updated
 
@@ -510,3 +523,322 @@ class TestStrategicOptimiser:
         assert opt.strategy_used.value == "case_2_solve_max_gain_under_budget"
         assert opt.solution_cost == 7787.068
         assert opt.solution_gain == 28.8
+
+
+class TestCheckNeedsVentilation:
+
+    def measure_types_includes_ventilation_no_existing_ventilation(self):
+        property_measure_types = {'mechanical_ventilation', 'cavity_wall_insulation', 'suspended_floor_insulation',
+                                  'secondary_heating', 'loft_insulation', 'heating', 'low_energy_lighting'}
+
+        measures_needing_ventilation = ['internal_wall_insulation', 'external_wall_insulation',
+                                        'cavity_wall_insulation']
+
+        has_ventilation = False
+
+        ventilation_included = True
+
+        result = optimiser_functions.check_needs_ventilation(
+            property_measure_types, measures_needing_ventilation, has_ventilation,
+            ventilation_included
+        )
+
+        assert result == True
+
+    def measure_types_includes_ventilation_existing_ventilation(self):
+        property_measure_types = {'mechanical_ventilation', 'cavity_wall_insulation', 'suspended_floor_insulation',
+                                  'secondary_heating', 'loft_insulation', 'heating', 'low_energy_lighting'}
+
+        measures_needing_ventilation = ['internal_wall_insulation', 'external_wall_insulation',
+                                        'cavity_wall_insulation']
+
+        has_ventilation = True
+
+        ventilation_included = True
+
+        result = optimiser_functions.check_needs_ventilation(
+            property_measure_types, measures_needing_ventilation, has_ventilation,
+            ventilation_included
+        )
+
+        assert result == False
+
+    def measure_types_includes_ventilation_existing_ventilation(self):
+        property_measure_types_without_ventilation = {
+            'cavity_wall_insulation', 'suspended_floor_insulation',
+            'secondary_heating', 'loft_insulation', 'heating',
+            'low_energy_lighting'
+        }
+
+        measures_needing_ventilation = ['internal_wall_insulation', 'external_wall_insulation',
+                                        'cavity_wall_insulation']
+
+        has_ventilation = False
+
+        ventilation_included = True
+
+        result = optimiser_functions.check_needs_ventilation(
+            property_measure_types_without_ventilation, measures_needing_ventilation, has_ventilation,
+            ventilation_included
+        )
+
+        assert result == False
+
+
+class TestOptimiseWithScenarios:
+
+    def test_zero_gain(self, property_instance):
+        input_measures = [[{'id': '0_phase=0', 'cost': 16901.01977922431, 'gain': np.float64(2.0),
+                            'type': 'internal_wall_insulation+mechanical_ventilation', 'innovation_uplift': 0,
+                            'cost_minus_uplift': 16901.01977922431, 'raw_cost': 16341.019779224309,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0}],
+                          [{'id': '1_phase=1', 'cost': 1197.0, 'gain': 0, 'type': 'loft_insulation',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 1197.0, 'raw_cost': 1197.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0},
+                           {'id': '2_phase=1', 'cost': 1026.0, 'gain': 0, 'type': 'loft_insulation',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 1026.0, 'raw_cost': 1026.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0},
+                           {'id': '3_phase=1', 'cost': 855.0, 'gain': 0, 'type': 'loft_insulation',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 855.0, 'raw_cost': 855.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0}],
+                          [{'id': '5_phase=3', 'cost': 5343.75, 'gain': 1, 'type': 'suspended_floor_insulation',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5343.75, 'raw_cost': 5343.75,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0}],
+                          [{'id': '6_phase=4', 'cost': 1009.5600000000001, 'gain': np.float64(0.9000000000000057),
+                            'type': 'time_temperature_zone_control', 'innovation_uplift': 0,
+                            'cost_minus_uplift': 1009.5600000000001, 'raw_cost': 1009.5600000000001,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0},
+                           {'id': '7_phase=4', 'cost': 18979.9, 'gain': np.float64(6.9), 'type': 'air_source_heat_pump',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 18979.9, 'raw_cost': 18979.9,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 0}],
+                          [{'id': '8_phase=5', 'cost': 5420.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5420.0, 'raw_cost': 5420.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 3.6},
+                           {'id': '9_phase=5', 'cost': 6210.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6210.0, 'raw_cost': 6210.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.6},
+                           {'id': '10_phase=5', 'cost': 6820.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6820.0, 'raw_cost': 6820.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.6},
+                           {'id': '11_phase=5', 'cost': 7202.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7202.0, 'raw_cost': 7202.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 3.915},
+                           {'id': '12_phase=5', 'cost': 6495.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6495.0, 'raw_cost': 6495.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 3.92},
+                           {'id': '13_phase=5', 'cost': 7285.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7285.0, 'raw_cost': 7285.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.92},
+                           {'id': '14_phase=5', 'cost': 7895.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7895.0, 'raw_cost': 7895.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.92},
+                           {'id': '15_phase=5', 'cost': 5520.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5520.0, 'raw_cost': 5520.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.0},
+                           {'id': '16_phase=5', 'cost': 6310.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6310.0, 'raw_cost': 6310.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.0},
+                           {'id': '17_phase=5', 'cost': 6920.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6920.0, 'raw_cost': 6920.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.0},
+                           {'id': '18_phase=5', 'cost': 5840.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5840.0, 'raw_cost': 5840.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 5.2},
+                           {'id': '19_phase=5', 'cost': 6630.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6630.0, 'raw_cost': 6630.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 5.2},
+                           {'id': '20_phase=5', 'cost': 7240.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7240.0, 'raw_cost': 7240.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 5.2},
+                           {'id': '21_phase=5', 'cost': 8630.0, 'gain': np.float64(14.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8630.0, 'raw_cost': 8630.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 5.655},
+                           {'id': '22_phase=5', 'cost': 7660.0, 'gain': np.float64(14.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7660.0, 'raw_cost': 7660.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 5.66},
+                           {'id': '23_phase=5', 'cost': 8470.0, 'gain': np.float64(14.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8470.0, 'raw_cost': 8470.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 5.66},
+                           {'id': '24_phase=5', 'cost': 9090.0, 'gain': np.float64(14.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 9090.0, 'raw_cost': 9090.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 5.66},
+                           {'id': '25_phase=5', 'cost': 7240.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7240.0, 'raw_cost': 7240.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.79},
+                           {'id': '26_phase=5', 'cost': 8050.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8050.0, 'raw_cost': 8050.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.79},
+                           {'id': '27_phase=5', 'cost': 8660.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8660.0, 'raw_cost': 8660.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.79},
+                           {'id': '28_phase=5', 'cost': 5740.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5740.0, 'raw_cost': 5740.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.8},
+                           {'id': '29_phase=5', 'cost': 6530.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6530.0, 'raw_cost': 6530.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.8},
+                           {'id': '30_phase=5', 'cost': 7140.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7140.0, 'raw_cost': 7140.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.8},
+                           {'id': '31_phase=5', 'cost': 8360.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8360.0, 'raw_cost': 8360.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 5.22},
+                           {'id': '32_phase=5', 'cost': 7470.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7470.0, 'raw_cost': 7470.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 5.22},
+                           {'id': '33_phase=5', 'cost': 8280.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8280.0, 'raw_cost': 8280.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 5.22},
+                           {'id': '34_phase=5', 'cost': 8890.0, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8890.0, 'raw_cost': 8890.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 5.22},
+                           {'id': '35_phase=5', 'cost': 5892.21, 'gain': np.float64(13.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5892.21, 'raw_cost': 5892.21,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 5.34},
+                           {'id': '36_phase=5', 'cost': 5320.0, 'gain': np.float64(8.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5320.0, 'raw_cost': 5320.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 3.2},
+                           {'id': '37_phase=5', 'cost': 6110.0, 'gain': np.float64(8.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6110.0, 'raw_cost': 6110.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.2},
+                           {'id': '38_phase=5', 'cost': 6720.0, 'gain': np.float64(8.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6720.0, 'raw_cost': 6720.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.2},
+                           {'id': '39_phase=5', 'cost': 6932.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6932.0, 'raw_cost': 6932.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 3.48},
+                           {'id': '40_phase=5', 'cost': 6295.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6295.0, 'raw_cost': 6295.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 3.48},
+                           {'id': '41_phase=5', 'cost': 7085.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7085.0, 'raw_cost': 7085.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.48},
+                           {'id': '42_phase=5', 'cost': 7695.0, 'gain': np.float64(9.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7695.0, 'raw_cost': 7695.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 3.48},
+                           {'id': '43_phase=5', 'cost': 5640.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5640.0, 'raw_cost': 5640.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.4},
+                           {'id': '44_phase=5', 'cost': 6430.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6430.0, 'raw_cost': 6430.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.4},
+                           {'id': '45_phase=5', 'cost': 7040.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7040.0, 'raw_cost': 7040.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.4},
+                           {'id': '46_phase=5', 'cost': 8090.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8090.0, 'raw_cost': 8090.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.785},
+                           {'id': '47_phase=5', 'cost': 7240.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7240.0, 'raw_cost': 7240.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.79},
+                           {'id': '48_phase=5', 'cost': 8050.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8050.0, 'raw_cost': 8050.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.79},
+                           {'id': '49_phase=5', 'cost': 8660.0, 'gain': np.float64(12.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8660.0, 'raw_cost': 8660.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.79},
+                           {'id': '50_phase=5', 'cost': 5520.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5520.0, 'raw_cost': 5520.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.0},
+                           {'id': '51_phase=5', 'cost': 6310.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6310.0, 'raw_cost': 6310.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.0},
+                           {'id': '52_phase=5', 'cost': 6920.0, 'gain': np.float64(10.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6920.0, 'raw_cost': 6920.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.0},
+                           {'id': '53_phase=5', 'cost': 7820.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7820.0, 'raw_cost': 7820.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.35},
+                           {'id': '54_phase=5', 'cost': 6675.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6675.0, 'raw_cost': 6675.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.35},
+                           {'id': '55_phase=5', 'cost': 7485.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7485.0, 'raw_cost': 7485.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.35},
+                           {'id': '56_phase=5', 'cost': 8095.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 8095.0, 'raw_cost': 8095.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.35},
+                           {'id': '57_phase=5', 'cost': 5640.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5640.0, 'raw_cost': 5640.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.4},
+                           {'id': '58_phase=5', 'cost': 6430.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 6430.0, 'raw_cost': 6430.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.4},
+                           {'id': '59_phase=5', 'cost': 7040.0, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 7040.0, 'raw_cost': 7040.0,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': True, 'array_size': 4.4},
+                           {'id': '60_phase=5', 'cost': 5692.21, 'gain': np.float64(11.0), 'type': 'solar_pv',
+                            'innovation_uplift': 0, 'cost_minus_uplift': 5692.21, 'raw_cost': 5692.21,
+                            'partial_project_funding': 0, 'partial_project_score': 0, 'uplift_project_score': 0,
+                            'already_installed': False, 'has_battery': False, 'array_size': 4.45}]]
+
+        solutions = optimise_with_scenarios(
+            p=property_instance,
+            input_measures=input_measures,
+            budget=None,
+            target_gain=0,
+            enforce_heat_pump_insulation=True,
+            enforce_fabric_first=False,
+            already_installed_sap=0,  # To be passed to output
+        )
+
+        assert solutions.empty

recommendations/tests/test_recommendations.py

@@ -373,7 +373,7 @@ def test_filter_phase_adjustment(input_data, expected):
     "sap_impact, limit, expected",
     [
         (1.0, -4, True),  # positive SAP not allowed
-        (0.0, -4, True),  # zero not allowed
+        (0.0, -4, False),  # zero is allowed
         (-1.0, -4, False),  # valid range
         (-3.9, -4, False),  # valid range
         (-4.0, -4, False),  # exact lower bound allowed
@@ -401,7 +401,7 @@ def test_adjust_ventilation_sap(sap_impact, limit, expected):
     ) == expected
 
 
-def test_get_previous_phase_values_starting_phase(property_instance):
+def test_get_previous_phase_values_phase_0_starting_phase_0(property_instance):
     result = Recommendations._get_previous_phase_values(
         rec_phase=0,
         starting_phase=0,
@@ -411,6 +411,7 @@ def test_get_previous_phase_values_starting_phase(property_instance):
 
     assert result == {
         "sap": 65.0,
+        "sap_prediction": 65.0,
         "carbon": 2.4,
         "heat_demand": 284.0,
     }
@@ -441,8 +442,8 @@ def test_get_previous_phase_values_single_rep(property_instance):
 
 def test_get_previous_phase_values_median(property_instance):
     impact_summary = [
-        {"phase": 1, "representative": True, "sap": 70, "carbon": 2.0, "heat_demand": 250},
-        {"phase": 1, "representative": True, "sap": 74, "carbon": 1.6, "heat_demand": 230},
+        {"phase": 1, "representative": True, "sap": 70, "carbon": 2.0, "heat_demand": 250, "sap_prediction": 70},
+        {"phase": 1, "representative": True, "sap": 74, "carbon": 1.6, "heat_demand": 230, "sap_prediction": 74},
     ]
 
     result = Recommendations._get_previous_phase_values(
@@ -1476,7 +1477,9 @@ def test_lighting_and_loft_adjustment_combined(property_instance, heat_demand_pr
 
     assert adjustments2 == [
         {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)},
-        {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)}
+        {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)},
+        {'recommendation_id': '5_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0)},
+        {'recommendation_id': '6_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0000000000000027)}
     ]
 
 
@@ -1499,7 +1502,8 @@ def test_mechanical_ventilation_sap_floor(property_instance):
             previous_phase_values=previous_phase_values,
             current_phase_values=current_phase_values,
             adjustments=adjustments,
-            property_instance=property_instance
+            property_instance=property_instance,
+            model_predicted_sap=0
         )
     )
 
@@ -1538,7 +1542,8 @@ def test_mechanical_ventilation_no_floor_adjustment(property_instance):
             previous_phase_values=previous_phase_values,
             current_phase_values=current_phase_values,
             adjustments=adjustments,
-            property_instance=property_instance
+            property_instance=property_instance,
+            model_predicted_sap=0
         )
     )
 
@@ -1570,7 +1575,8 @@ def test_mechanical_ventilation_exactly_one_no_adjustment(property_instance):
             previous_phase_values=previous_phase_values,
             current_phase_values=current_phase_values,
             adjustments=adjustments,
-            property_instance=property_instance
+            property_instance=property_instance,
+            model_predicted_sap=0
         )
     )
 
@@ -1578,3 +1584,182 @@ def test_mechanical_ventilation_exactly_one_no_adjustment(property_instance):
     assert updated_adjustments == []
     assert updated_current["sap"] == 1.0
     assert updated_impact["sap"] == -1.0
+
+
+def test_mechanical_ventilation_sap_zero_no_adjustment(property_instance):
+    # Test when SAP = 0
+    rec = {
+        "type": "mechanical_ventilation",
+        "recommendation_id": "mv_test",
+        "phase": 1,
+    }
+
+    previous_phase_values = {'phase': 0, 'representative': True, 'recommendation_id': '0_phase=0',
+                             'measure_type': 'flat_roof_insulation', 'sap': 68.0, 'carbon': np.float64(0.5),
+                             'heat_demand': np.float64(300.1), 'sap_prediction': np.float64(71.7)}
+    current_phase_values = {'sap': 68.0, 'carbon': np.float64(0.5), 'heat_demand': np.float64(307.0)}
+    property_phase_impact = {'sap': 0, 'carbon': 0, 'heat_demand': np.float64(-6.899999999999977)}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec=rec,
+            property_phase_impact=property_phase_impact,
+            previous_phase_values=previous_phase_values,
+            current_phase_values=current_phase_values,
+            adjustments=adjustments,
+            property_instance=property_instance,
+            model_predicted_sap=0
+        )
+    )
+
+    # SAP is already at 0 → no adjustment expected
+    assert updated_adjustments == []
+    assert updated_current["sap"] == 68.0
+    assert updated_impact["sap"] == 0
+
+
+def test_mv_valid_negative_no_adjustment(property_instance):
+    rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1}
+
+    previous = {"sap": 70.0}
+    current = {"sap": 67.0}
+    impact = {"sap": -3.0, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert updated_adjustments == []
+    assert updated_current["sap"] == 67.0
+    assert updated_impact["sap"] == -3.0
+
+
+def test_mv_zero_impact_allowed(property_instance):
+    rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1}
+
+    previous = {"sap": 68.0, "sap_prediction": 71.7}
+    current = {"sap": 68.0}
+    impact = {"sap": 0.0, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert updated_adjustments == []
+    assert updated_current["sap"] == 68.0
+    assert updated_impact["sap"] == 0.0
+
+
+def test_mv_positive_impact_corrected(property_instance):
+    rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1}
+
+    previous = {"sap": 60.0}
+    current = {"sap": 61.0}
+    impact = {"sap": 1.0, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert len(updated_adjustments) == 1
+    assert updated_current["sap"] == previous["sap"] + updated_impact["sap"]
+    assert updated_impact["sap"] <= 0
+
+
+def test_mv_below_lower_bound_corrected(property_instance):
+    rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1}
+
+    previous = {"sap": 70.0}
+    current = {"sap": 64.0}
+    impact = {"sap": -6.0, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert len(updated_adjustments) == 1
+    assert updated_impact["sap"] >= -4
+
+
+def test_mv_floor_triggered(property_instance):
+    rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1}
+
+    previous = {"sap": 2.0}
+    current = {"sap": 0.5}
+    impact = {"sap": -1.5, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert updated_current["sap"] == 1.0
+    assert updated_adjustments[0]["sap_adjustment"] > 0
+
+
+def test_mv_exactly_one_no_floor(property_instance):
+    rec = {"type": "mechanical_ventilation", "recommendation_id": "mv", "phase": 1}
+
+    previous = {"sap": 2.0}
+    current = {"sap": 1.0}
+    impact = {"sap": -1.0, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert updated_adjustments == []
+    assert updated_current["sap"] == 1.0
+
+
+def test_lighting_no_cap(property_instance):
+    rec = {"type": "low_energy_lighting", "recommendation_id": "led", "phase": 1,
+           "co2_equivalent_savings": 0}
+
+    previous = {"sap": 60.0, "carbon": 2.0}
+    current = {"sap": 61.0, "carbon": 2.0}
+    impact = {"sap": 1.0, "carbon": 0, "heat_demand": 0}
+    adjustments = []
+
+    updated_impact, updated_current, updated_adjustments = (
+        Recommendations._apply_measure_specific_rules(
+            rec, impact, previous, current, adjustments, property_instance, 0
+        )
+    )
+
+    assert updated_adjustments == []
+
+
+def test_filter_phase_adjustments():
+    example_adjustments = [
+        {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)},
+        {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)},
+        {'recommendation_id': '5_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0)},
+        {'recommendation_id': '6_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0000000000000027)}
+    ]
+
+    res = Recommendations._filter_phase_adjustment(example_adjustments)
+
+    assert res == [
+        {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)},
+        {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)},
+        {'recommendation_id': '6_phase=3', 'phase': 3, 'sap_adjustment': np.float64(1.0000000000000027)}
+    ]