From fac418adbe2de14ad29b40abc74cab73653c4ba3 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Tue, 17 Feb 2026 15:25:51 +0000 Subject: [PATCH 01/21] Don't re-get scenarios for every plan --- .../db/functions/recommendations_functions.py | 7 +++++++ backend/categorisation/processor.py | 21 ++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index e690991a..aa966fbb 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -625,6 +625,13 @@ def get_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]: 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_scenario(scenario_id: int) -> Optional[ScenarioModel]: stmt = select(ScenarioModel).where(ScenarioModel.id == scenario_id) with db_read_session() as session: diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index 7c5698b7..d2bdbef0 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -4,7 +4,7 @@ from typing import Dict, List from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, get_plans_by_portfolio_id, - get_scenario, + get_scenarios_by_portfolio_id, ) from backend.app.db.models.recommendations import PlanModel, ScenarioModel from backend.app.domain.classes.plan import Plan @@ -15,7 +15,7 @@ logger = setup_logger() def process_portfolio(portfolio_id: int) -> None: - print(f"Processing portfolio {portfolio_id}") + logger.info(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) @@ -29,22 +29,27 @@ def process_portfolio(portfolio_id: int) -> None: 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") - plans: List[Plan] = [] + plan_models = get_plans_by_portfolio_id(portfolio_id) + scenarios: List[ScenarioModel] = get_scenarios_by_portfolio_id(portfolio_id) + + if not scenarios: + raise Exception(f"No scenarios found for Portfolio {portfolio_id}") + for model in plan_models: - if not model.scenario_id: + + scenario_model = next((s for s in scenarios if s.id == model.scenario_id)) + if not scenario_model: 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") + logger.info("Successfully mapped plan and scenario to domain object") + logger.info(f"Got {len(plans)} plans from database") return plans From 3a5df1a1f3f5e15c9a6bd353d8f957a7d3512e6d Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 09:05:41 +0000 Subject: [PATCH 02/21] Better logging --- backend/categorisation/processor.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index d2bdbef0..97e4c5ad 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -47,9 +47,11 @@ def _load_plans_for_portfolio(portfolio_id: int) -> List[Plan]: plans.append( Plan.from_sqlalchemy(model, Scenario.from_sqlalchemy(scenario_model)) ) - logger.info("Successfully mapped plan and scenario to domain object") + logger.debug( + f"Successfully mapped plan {model.id} and scenario {scenario_model.id} to domain object" + ) - logger.info(f"Got {len(plans)} plans from database") + logger.debug(f"Got {len(plans)} plans from database") return plans @@ -83,6 +85,9 @@ def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None: for plan in plans: should_be_default: bool = plan.id == cheapest_plan.id if plan.record.is_default != should_be_default: + logger.info( + f"Setting Plan {plan.id} (Scenario Name: {plan.scenario.record.name}) to is_default: {should_be_default}" + ) plan.set_default(should_be_default) plans_to_update.append(plan) @@ -96,3 +101,7 @@ def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None: scenario_models.append(scenario_model) bulk_update_plans(plan_models, scenario_models) + logger.info("Successfully updated Plan default values") + + else: + logger.info("All plan default values already correct. Not udpating") From 9a177065b611c0fded57070554701f010649b852 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 10:06:40 +0000 Subject: [PATCH 03/21] =?UTF-8?q?allow=20plan=20priority=20to=20be=20speci?= =?UTF-8?q?fied=20for=20plans=20with=20identical=20ouput=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../categorisation_trigger_request.py | 3 + backend/categorisation/processor.py | 14 ++- .../tests/test_prioritised_plan_selected.py | 88 +++++++++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 backend/categorisation/tests/test_prioritised_plan_selected.py diff --git a/backend/categorisation/categorisation_trigger_request.py b/backend/categorisation/categorisation_trigger_request.py index 9ef1d106..aa2b8ed3 100644 --- a/backend/categorisation/categorisation_trigger_request.py +++ b/backend/categorisation/categorisation_trigger_request.py @@ -1,5 +1,8 @@ +from typing import List, Optional from pydantic import BaseModel class CategorisationTriggerRequest(BaseModel): portfolio_id: int + + plan_priority_order: Optional[List[int]] diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index 97e4c5ad..539f7a68 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -1,5 +1,5 @@ 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, @@ -14,7 +14,9 @@ from utils.logger import setup_logger logger = setup_logger() -def process_portfolio(portfolio_id: int) -> None: +def process_portfolio( + portfolio_id: int, plan_priority_order: Optional[List[int]] = [] +) -> None: logger.info(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) @@ -24,7 +26,9 @@ def process_portfolio(portfolio_id: int) -> None: if not property_plans: raise ValueError(f"No plans for property {uprn}") - cheapest_plan = _choose_cheapest_relevant_plan(property_plans) + cheapest_plan = _choose_cheapest_relevant_plan( + property_plans, plan_priority_order + ) _update_default_flags(property_plans, cheapest_plan) @@ -64,7 +68,9 @@ def _group_plans_by_property(plans: List[Plan]) -> Dict[int, List[Plan]]: return grouped -def _choose_cheapest_relevant_plan(plans: List[Plan]) -> Plan: +def _choose_cheapest_relevant_plan( + plans: List[Plan], plan_priority_order: Optional[List[int]] = [] +) -> Plan: plans_to_consider: List[Plan] = [p for p in plans if p.is_compliant] or plans def plan_cost(plan: Plan) -> float: diff --git a/backend/categorisation/tests/test_prioritised_plan_selected.py b/backend/categorisation/tests/test_prioritised_plan_selected.py new file mode 100644 index 00000000..03bca666 --- /dev/null +++ b/backend/categorisation/tests/test_prioritised_plan_selected.py @@ -0,0 +1,88 @@ +from datetime import datetime +from typing import List +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() + + +@pytest.fixture +def identical_plan_record(created_at_datetime: datetime, default: bool) -> PlanRecord: + return PlanRecord( + property_id=1, + portfolio_id=1, + created_at=created_at_datetime, + is_default=default, + post_epc_rating=Epc.C, + cost_of_works=500.0, + ) + + +def make_plan_record(created_at_datetime: datetime, default: bool) -> PlanRecord: + return PlanRecord( + property_id=1, + portfolio_id=1, + created_at=created_at_datetime, + is_default=default, + post_epc_rating=Epc.C, + cost_of_works=500.0, + ) + + +def test_prioritised_plan_selected(created_at_datetime: datetime) -> None: + # arrange + epc_c_scenario_record = ScenarioRecord( + name="EPC C", + created_at=created_at_datetime, + housing_type="", + goal=PortfolioGoal.INCREASING_EPC, + goal_value="C", + trigger_file_path="", + multi_plan=False, + is_default=True, + ) + epc_c_scenario = Scenario(record=epc_c_scenario_record, id=1) + epc_c_plan = Plan( + record=make_plan_record(created_at_datetime, True), + scenario=epc_c_scenario, + id=1, + ) + + minor_works_scenario_record = ScenarioRecord( + name="EPC C - Minor Works", + created_at=created_at_datetime, + housing_type="", + goal=PortfolioGoal.INCREASING_EPC, + goal_value="C", + trigger_file_path="", + multi_plan=False, + is_default=False, + ) + minor_works_scenario = Scenario(record=minor_works_scenario_record, id=2) + minor_works_plan = Plan( + record=make_plan_record(created_at_datetime, False), + scenario=minor_works_scenario, + id=2, + ) + + plan_priority_order: List[int] = [2, 1] + + expected_default_plan_id = 2 + + # act + actual_default_plan = _choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + plan_priority_order=plan_priority_order, + ) + + # assert + assert actual_default_plan.id == expected_default_plan_id From 508f3f285934908f5ebeab2b9437dac2c108184e Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 10:40:50 +0000 Subject: [PATCH 04/21] make choose cheapest relevant plan method public as it's called from outside the module --- backend/categorisation/processor.py | 36 +++++++++---------- .../tests/test_prioritised_plan_selected.py | 4 +-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index 539f7a68..02116d61 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -26,12 +26,29 @@ def process_portfolio( if not property_plans: raise ValueError(f"No plans for property {uprn}") - cheapest_plan = _choose_cheapest_relevant_plan( + cheapest_plan = choose_cheapest_relevant_plan( property_plans, plan_priority_order ) _update_default_flags(property_plans, cheapest_plan) +def choose_cheapest_relevant_plan( + plans: List[Plan], plan_priority_order: Optional[List[int]] = [] +) -> 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 _load_plans_for_portfolio(portfolio_id: int) -> List[Plan]: plans: List[Plan] = [] @@ -68,23 +85,6 @@ def _group_plans_by_property(plans: List[Plan]) -> Dict[int, List[Plan]]: return grouped -def _choose_cheapest_relevant_plan( - plans: List[Plan], plan_priority_order: Optional[List[int]] = [] -) -> 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] = [] diff --git a/backend/categorisation/tests/test_prioritised_plan_selected.py b/backend/categorisation/tests/test_prioritised_plan_selected.py index 03bca666..eb41194c 100644 --- a/backend/categorisation/tests/test_prioritised_plan_selected.py +++ b/backend/categorisation/tests/test_prioritised_plan_selected.py @@ -7,7 +7,7 @@ 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 +from backend.categorisation.processor import choose_cheapest_relevant_plan @pytest.fixture @@ -79,7 +79,7 @@ def test_prioritised_plan_selected(created_at_datetime: datetime) -> None: expected_default_plan_id = 2 # act - actual_default_plan = _choose_cheapest_relevant_plan( + actual_default_plan = choose_cheapest_relevant_plan( plans=[epc_c_plan, minor_works_plan], plan_priority_order=plan_priority_order, ) From b916551921f3e5590fd1d7caf7270370348361de Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 11:20:08 +0000 Subject: [PATCH 05/21] =?UTF-8?q?allow=20plan=20priority=20to=20be=20speci?= =?UTF-8?q?fied=20for=20plans=20with=20identical=20ouput=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/categorisation/processor.py | 37 ++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index 02116d61..184ccac2 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -1,6 +1,8 @@ from collections import defaultdict from typing import Dict, List, Optional +from sqlalchemy import Tuple + from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, get_plans_by_portfolio_id, @@ -33,20 +35,43 @@ def process_portfolio( def choose_cheapest_relevant_plan( - plans: List[Plan], plan_priority_order: Optional[List[int]] = [] + plans: List[Plan], plan_priority_order: Optional[List[int]] = None ) -> Plan: - plans_to_consider: List[Plan] = [p for p in plans if p.is_compliant] or plans + plan_priority_order = plan_priority_order or [] - def plan_cost(plan: Plan) -> float: - return ( + 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.record.cost_of_works if plan.record.cost_of_works is not None else float("inf") ) + for plan in eligible_plans + ) - cheapest_plan = min(plans_to_consider, key=plan_cost) + cheapest_plans: List[Plan] = [ + plan + for plan in eligible_plans + if (plan.record.cost_of_works or float("inf")) == min_cost + ] - return cheapest_plan + for priority_plan_id in plan_priority_order: + for plan in cheapest_plans: + if plan.id == priority_plan_id: + return plan + + return cheapest_plans[0] def _load_plans_for_portfolio(portfolio_id: int) -> List[Plan]: From bfb0d79da6c7d7e0f8e9465683c14e7141e0b1ea Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 11:27:09 +0000 Subject: [PATCH 06/21] =?UTF-8?q?Cheapest=20compliant=20plan=20selected=20?= =?UTF-8?q?even=20when=20not=20in=20the=20priority=20list=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../tests/test_prioritised_plan_selected.py | 100 ++++++++++-------- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/backend/categorisation/tests/test_prioritised_plan_selected.py b/backend/categorisation/tests/test_prioritised_plan_selected.py index eb41194c..5424dd5e 100644 --- a/backend/categorisation/tests/test_prioritised_plan_selected.py +++ b/backend/categorisation/tests/test_prioritised_plan_selected.py @@ -15,67 +15,73 @@ def created_at_datetime() -> datetime: return datetime.now() -@pytest.fixture -def identical_plan_record(created_at_datetime: datetime, default: bool) -> PlanRecord: +def make_plan_record( + created_at: datetime, default: bool, cost_of_works: float = 500.0 +) -> PlanRecord: return PlanRecord( property_id=1, portfolio_id=1, - created_at=created_at_datetime, + created_at=created_at, is_default=default, post_epc_rating=Epc.C, - cost_of_works=500.0, + cost_of_works=cost_of_works, ) -def make_plan_record(created_at_datetime: datetime, default: bool) -> PlanRecord: - return PlanRecord( - property_id=1, - portfolio_id=1, - created_at=created_at_datetime, - is_default=default, - post_epc_rating=Epc.C, - cost_of_works=500.0, +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=1 if is_default else 2) + + +def make_plan( + created_at: datetime, default: bool, cost_of_works: 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_plan_selected(created_at_datetime: datetime) -> None: # arrange - epc_c_scenario_record = ScenarioRecord( - name="EPC C", - created_at=created_at_datetime, - housing_type="", - goal=PortfolioGoal.INCREASING_EPC, - goal_value="C", - trigger_file_path="", - multi_plan=False, - is_default=True, - ) - epc_c_scenario = Scenario(record=epc_c_scenario_record, id=1) - epc_c_plan = Plan( - record=make_plan_record(created_at_datetime, True), - scenario=epc_c_scenario, - id=1, - ) - - minor_works_scenario_record = ScenarioRecord( - name="EPC C - Minor Works", - created_at=created_at_datetime, - housing_type="", - goal=PortfolioGoal.INCREASING_EPC, - goal_value="C", - trigger_file_path="", - multi_plan=False, - is_default=False, - ) - minor_works_scenario = Scenario(record=minor_works_scenario_record, id=2) - minor_works_plan = Plan( - record=make_plan_record(created_at_datetime, False), - scenario=minor_works_scenario, - id=2, - ) - + 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") plan_priority_order: List[int] = [2, 1] - + expected_default_plan_id = 2 + + # act + actual_default_plan = choose_cheapest_relevant_plan( + plans=[epc_c_plan, minor_works_plan], + plan_priority_order=plan_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" + ) + plan_priority_order: List[int] = [1, 3] expected_default_plan_id = 2 # act From cc901d999b538e1743a32a00ab474460f3fd7bc5 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 11:54:55 +0000 Subject: [PATCH 07/21] option to only consider a specific list of plans --- .../db/functions/recommendations_functions.py | 7 +++++ .../categorisation_trigger_request.py | 3 +- backend/categorisation/handler/handler.py | 6 +++- backend/categorisation/processor.py | 30 +++++++++++++++---- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index aa966fbb..d4c3fcb9 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -625,6 +625,13 @@ def get_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]: return session_any.exec(stmt).scalars().all() +def get_plans_by_ids(ids: List[int]) -> List[PlanModel]: + stmt = select(PlanModel).where(PlanModel.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_scenarios_by_portfolio_id(portfolio_id: int) -> List[ScenarioModel]: stmt = select(ScenarioModel).where(ScenarioModel.portfolio_id == portfolio_id) with db_read_session() as session: diff --git a/backend/categorisation/categorisation_trigger_request.py b/backend/categorisation/categorisation_trigger_request.py index aa2b8ed3..46ce6f1c 100644 --- a/backend/categorisation/categorisation_trigger_request.py +++ b/backend/categorisation/categorisation_trigger_request.py @@ -5,4 +5,5 @@ from pydantic import BaseModel class CategorisationTriggerRequest(BaseModel): portfolio_id: int - plan_priority_order: Optional[List[int]] + plans_to_consider: Optional[List[int]] = None + plan_priority_order: Optional[List[int]] = None diff --git a/backend/categorisation/handler/handler.py b/backend/categorisation/handler/handler.py index 20076613..449c5ccf 100644 --- a/backend/categorisation/handler/handler.py +++ b/backend/categorisation/handler/handler.py @@ -20,7 +20,11 @@ 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.plans_to_consider, + payload.plan_priority_order, + ) except Exception as e: logger.error(f"Failed to process record: {e}") diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index 184ccac2..b7ddfc62 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -1,10 +1,9 @@ from collections import defaultdict from typing import Dict, List, Optional -from sqlalchemy import Tuple - from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, + get_plans_by_ids, get_plans_by_portfolio_id, get_scenarios_by_portfolio_id, ) @@ -17,10 +16,14 @@ logger = setup_logger() def process_portfolio( - portfolio_id: int, plan_priority_order: Optional[List[int]] = [] + portfolio_id: int, + plans_to_consider: Optional[List[int]] = None, + plan_priority_order: Optional[List[int]] = None, ) -> None: logger.info(f"Processing portfolio {portfolio_id}") - plans: List[Plan] = _load_plans_for_portfolio(portfolio_id) + + plans: List[Plan] = _load_plans_for_portfolio(portfolio_id, plans_to_consider) + plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans) for uprn, property_plans in plans_by_property.items(): @@ -74,10 +77,25 @@ def choose_cheapest_relevant_plan( return cheapest_plans[0] -def _load_plans_for_portfolio(portfolio_id: int) -> List[Plan]: +def _load_plans_for_portfolio( + portfolio_id: int, plans_to_consider: Optional[List[int]] = None +) -> List[Plan]: + + if plans_to_consider: + if len(plans_to_consider) < 2: + raise ValueError("Cannot run auto categorisation for fewer than 2 plans") + + logger.info(f"Getting {len(plans_to_consider)} Plans") + plan_models: List[PlanModel] = get_plans_by_ids(plans_to_consider) + + else: + logger.info( + f"No list of Plans to consider provided. Getting all Plans for portfolio {portfolio_id}" + ) + plan_models: List[PlanModel] = get_plans_by_portfolio_id(portfolio_id) + plans: List[Plan] = [] - plan_models = get_plans_by_portfolio_id(portfolio_id) scenarios: List[ScenarioModel] = get_scenarios_by_portfolio_id(portfolio_id) if not scenarios: From 3dbe118b38064b6e93884b2e80ba6a92ab082365 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 13:29:16 +0000 Subject: [PATCH 08/21] additional logs in handler for local testing --- backend/categorisation/handler/handler.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/categorisation/handler/handler.py b/backend/categorisation/handler/handler.py index 449c5ccf..ee0e7a7d 100644 --- a/backend/categorisation/handler/handler.py +++ b/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"]) @@ -27,4 +32,5 @@ def handler(event: Mapping[str, Any], context: Any) -> None: ) except Exception as e: + logger.info("Handler exception") logger.error(f"Failed to process record: {e}") From f7fe7132c7a1f2a86eed8b239070584723ce0a88 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 15:12:24 +0000 Subject: [PATCH 09/21] docker compose for running lambdas locally --- backend/docker-compose-local-lambdas.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 backend/docker-compose-local-lambdas.yml diff --git a/backend/docker-compose-local-lambdas.yml b/backend/docker-compose-local-lambdas.yml new file mode 100644 index 00000000..781f4955 --- /dev/null +++ b/backend/docker-compose-local-lambdas.yml @@ -0,0 +1,11 @@ +version: "3.9" + +services: + lambda: + build: + context: ../ + dockerfile: backend/categorisation/handler/Dockerfile + ports: + - "9000:8080" + env_file: + - ../.env \ No newline at end of file From 3f022ba5488499fe630739c52a71cb5a572d908d Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 15:13:24 +0000 Subject: [PATCH 10/21] rename service in docker compose --- backend/docker-compose-local-lambdas.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/docker-compose-local-lambdas.yml b/backend/docker-compose-local-lambdas.yml index 781f4955..50e9193b 100644 --- a/backend/docker-compose-local-lambdas.yml +++ b/backend/docker-compose-local-lambdas.yml @@ -1,7 +1,7 @@ version: "3.9" services: - lambda: + categorisation-lambda: build: context: ../ dockerfile: backend/categorisation/handler/Dockerfile From b4e3dc9f42a8d24674ef19a72f8efbe75e178610 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 15:19:36 +0000 Subject: [PATCH 11/21] add example request body --- backend/categorisation/categorisation_trigger_request.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/categorisation/categorisation_trigger_request.py b/backend/categorisation/categorisation_trigger_request.py index 46ce6f1c..9bd7d7c8 100644 --- a/backend/categorisation/categorisation_trigger_request.py +++ b/backend/categorisation/categorisation_trigger_request.py @@ -7,3 +7,6 @@ class CategorisationTriggerRequest(BaseModel): plans_to_consider: Optional[List[int]] = None plan_priority_order: Optional[List[int]] = None + + +# {"portfolio_id": 556, "plans_to_consider": [1589319,1589320], "plan_priority_order": [1589319,1589320]} From 478947b8da91320f55b5e452e9803c90dec7cc6d Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 16:12:23 +0000 Subject: [PATCH 12/21] move docker compose back to categorisation directory and define simple invoke script --- .../local_handler/docker-compose.yml | 11 ++++++++ .../local_handler/invoke_local_lambda.py | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 backend/categorisation/local_handler/docker-compose.yml create mode 100644 backend/categorisation/local_handler/invoke_local_lambda.py diff --git a/backend/categorisation/local_handler/docker-compose.yml b/backend/categorisation/local_handler/docker-compose.yml new file mode 100644 index 00000000..9529fdb2 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py new file mode 100644 index 00000000..9eb5adda --- /dev/null +++ b/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, + "plans_to_consider": [], + "plan_priority_order": [], + } + ) + } + ] +} + +response = requests.post(LAMBDA_URL, json=payload) + +print("Status code:", response.status_code) +print("Response:") +print(response.text) From 490c8946d721725b16a90d40903ff667757b86cb Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 17:11:00 +0000 Subject: [PATCH 13/21] Unset existing default before setting new one --- .../db/functions/recommendations_functions.py | 34 +++++++++++++++++++ .../local_handler/invoke_local_lambda.py | 2 +- backend/categorisation/processor.py | 16 +++++++-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index d4c3fcb9..3af9fd29 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -646,6 +646,40 @@ def get_scenario(scenario_id: int) -> Optional[ScenarioModel]: return session_any.exec(stmt).scalar_one_or_none() +def get_default_plan_ids_for_property(property_id: int) -> List[int]: + # This should in reality always return exactly 1 ID, but there's currently + # no database constraint to enforce that, so account for 0 or >1 + stmt = select(PlanModel.id).where( + PlanModel.property_id == property_id and PlanModel.is_default + ) + with db_read_session() as session: + session_any: Any = session # Typehint as Any to satisfy Pylance... + return session_any.exec(stmt).scalars().all() + + +def set_plan_and_scenario_default(plan_id: int, default: bool) -> bool: + with db_session() as session: + plan: PlanModel = session.get(PlanModel, plan_id) + if not plan: + return False + + scenario_id = plan.scenario_id + + plan_mapper: Mapper[Any] = inspect(PlanModel) + scenario_mapper: Mapper[Any] = inspect(ScenarioModel) + + plan_mappings: List[Dict[str, Any]] = [{"id": plan.id, "is_default": default}] + scenario_mappings: List[Dict[str, Any]] = [ + {"id": scenario_id, "is_default": default} + ] + + session.bulk_update_mappings(plan_mapper, plan_mappings) + session.bulk_update_mappings(scenario_mapper, scenario_mappings) + session.commit() + + return True + + def bulk_update_plans( plan_models: List[PlanModel], scenario_models: List[ScenarioModel], diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py index 9eb5adda..23e5fda2 100644 --- a/backend/categorisation/local_handler/invoke_local_lambda.py +++ b/backend/categorisation/local_handler/invoke_local_lambda.py @@ -10,7 +10,7 @@ payload = { "body": json.dumps( { "portfolio_id": 556, - "plans_to_consider": [], + "plans_to_consider": [1589319, 1589320], "plan_priority_order": [], } ) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index b7ddfc62..b07f1c3b 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -3,9 +3,11 @@ from typing import Dict, List, Optional from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, + get_default_plan_ids_for_property, get_plans_by_ids, get_plans_by_portfolio_id, get_scenarios_by_portfolio_id, + set_plan_and_scenario_default, ) from backend.app.db.models.recommendations import PlanModel, ScenarioModel from backend.app.domain.classes.plan import Plan @@ -26,15 +28,23 @@ def process_portfolio( plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans) - for uprn, property_plans in plans_by_property.items(): + for property_id, property_plans in plans_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, plan_priority_order ) - _update_default_flags(property_plans, cheapest_plan) + + # Unset existing default(s) in case they are outside the plans to consider + default_plan_ids: List[int] = get_default_plan_ids_for_property(property_id) + for id in default_plan_ids: + set_plan_and_scenario_default(id, False) + + _update_default_flags( + property_plans, cheapest_plan + ) # TODO: we have already unset existing default(s), so this method can probably be a bit simpler now def choose_cheapest_relevant_plan( From 475b3d0e1305a801857bf13dde0915d80482894e Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 17:19:38 +0000 Subject: [PATCH 14/21] input is list of scenarios to consider not list of plans --- .../db/functions/recommendations_functions.py | 7 +++++++ .../categorisation_trigger_request.py | 2 +- backend/categorisation/handler/handler.py | 2 +- .../local_handler/invoke_local_lambda.py | 2 +- backend/categorisation/processor.py | 19 +++++++++++-------- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index 3af9fd29..6f7dd41f 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -632,6 +632,13 @@ def get_plans_by_ids(ids: List[int]) -> List[PlanModel]: return session_any.exec(stmt).scalars().all() +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_scenarios_by_portfolio_id(portfolio_id: int) -> List[ScenarioModel]: stmt = select(ScenarioModel).where(ScenarioModel.portfolio_id == portfolio_id) with db_read_session() as session: diff --git a/backend/categorisation/categorisation_trigger_request.py b/backend/categorisation/categorisation_trigger_request.py index 9bd7d7c8..4b35f75c 100644 --- a/backend/categorisation/categorisation_trigger_request.py +++ b/backend/categorisation/categorisation_trigger_request.py @@ -5,7 +5,7 @@ from pydantic import BaseModel class CategorisationTriggerRequest(BaseModel): portfolio_id: int - plans_to_consider: Optional[List[int]] = None + scenarios_to_consider: Optional[List[int]] = None plan_priority_order: Optional[List[int]] = None diff --git a/backend/categorisation/handler/handler.py b/backend/categorisation/handler/handler.py index ee0e7a7d..dc10fa4e 100644 --- a/backend/categorisation/handler/handler.py +++ b/backend/categorisation/handler/handler.py @@ -27,7 +27,7 @@ def handler(event: Mapping[str, Any], context: Any) -> None: process_portfolio( payload.portfolio_id, - payload.plans_to_consider, + payload.scenarios_to_consider, payload.plan_priority_order, ) diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py index 23e5fda2..ce599ca9 100644 --- a/backend/categorisation/local_handler/invoke_local_lambda.py +++ b/backend/categorisation/local_handler/invoke_local_lambda.py @@ -10,7 +10,7 @@ payload = { "body": json.dumps( { "portfolio_id": 556, - "plans_to_consider": [1589319, 1589320], + "scenarios_to_consider": [1040, 1041], "plan_priority_order": [], } ) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index b07f1c3b..e017c069 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -6,6 +6,7 @@ from backend.app.db.functions.recommendations_functions import ( get_default_plan_ids_for_property, get_plans_by_ids, get_plans_by_portfolio_id, + get_plans_by_scenario_ids, get_scenarios_by_portfolio_id, set_plan_and_scenario_default, ) @@ -19,12 +20,12 @@ logger = setup_logger() def process_portfolio( portfolio_id: int, - plans_to_consider: Optional[List[int]] = None, + scenarios_to_consider: Optional[List[int]] = None, plan_priority_order: Optional[List[int]] = None, ) -> None: logger.info(f"Processing portfolio {portfolio_id}") - plans: List[Plan] = _load_plans_for_portfolio(portfolio_id, plans_to_consider) + plans: List[Plan] = _load_plans_for_portfolio(portfolio_id, scenarios_to_consider) plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans) @@ -88,15 +89,17 @@ def choose_cheapest_relevant_plan( def _load_plans_for_portfolio( - portfolio_id: int, plans_to_consider: Optional[List[int]] = None + portfolio_id: int, scenarios_to_consider: Optional[List[int]] = None ) -> List[Plan]: - if plans_to_consider: - if len(plans_to_consider) < 2: - raise ValueError("Cannot run auto categorisation for fewer than 2 plans") + if scenarios_to_consider: + if len(scenarios_to_consider) < 2: + raise ValueError( + "Cannot run auto categorisation for fewer than 2 scenarios" + ) - logger.info(f"Getting {len(plans_to_consider)} Plans") - plan_models: List[PlanModel] = get_plans_by_ids(plans_to_consider) + logger.info(f"Getting {len(scenarios_to_consider)} plans") + plan_models: List[PlanModel] = get_plans_by_scenario_ids(scenarios_to_consider) else: logger.info( From ce8c1d23e6c8081b85f5dc2d59f71ca62df6a14f Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 17:24:17 +0000 Subject: [PATCH 15/21] =?UTF-8?q?priority=20list=20is=20scenarios=20not=20?= =?UTF-8?q?plans=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../categorisation/categorisation_trigger_request.py | 2 +- backend/categorisation/handler/handler.py | 2 +- backend/categorisation/processor.py | 11 +++++------ .../tests/test_prioritised_plan_selected.py | 12 ++++++------ 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/backend/categorisation/categorisation_trigger_request.py b/backend/categorisation/categorisation_trigger_request.py index 4b35f75c..fbc2328b 100644 --- a/backend/categorisation/categorisation_trigger_request.py +++ b/backend/categorisation/categorisation_trigger_request.py @@ -6,7 +6,7 @@ class CategorisationTriggerRequest(BaseModel): portfolio_id: int scenarios_to_consider: Optional[List[int]] = None - plan_priority_order: Optional[List[int]] = None + scenario_priority_order: Optional[List[int]] = None # {"portfolio_id": 556, "plans_to_consider": [1589319,1589320], "plan_priority_order": [1589319,1589320]} diff --git a/backend/categorisation/handler/handler.py b/backend/categorisation/handler/handler.py index dc10fa4e..9fb235d5 100644 --- a/backend/categorisation/handler/handler.py +++ b/backend/categorisation/handler/handler.py @@ -28,7 +28,7 @@ def handler(event: Mapping[str, Any], context: Any) -> None: process_portfolio( payload.portfolio_id, payload.scenarios_to_consider, - payload.plan_priority_order, + payload.scenario_priority_order, ) except Exception as e: diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index e017c069..fd0d9c89 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -4,7 +4,6 @@ from typing import Dict, List, Optional from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, get_default_plan_ids_for_property, - get_plans_by_ids, get_plans_by_portfolio_id, get_plans_by_scenario_ids, get_scenarios_by_portfolio_id, @@ -21,7 +20,7 @@ logger = setup_logger() def process_portfolio( portfolio_id: int, scenarios_to_consider: Optional[List[int]] = None, - plan_priority_order: Optional[List[int]] = None, + scenario_priority_order: Optional[List[int]] = None, ) -> None: logger.info(f"Processing portfolio {portfolio_id}") @@ -35,7 +34,7 @@ def process_portfolio( raise ValueError(f"No plans for property {property_id}") cheapest_plan = choose_cheapest_relevant_plan( - property_plans, plan_priority_order + property_plans, scenario_priority_order ) # Unset existing default(s) in case they are outside the plans to consider @@ -49,9 +48,9 @@ def process_portfolio( def choose_cheapest_relevant_plan( - plans: List[Plan], plan_priority_order: Optional[List[int]] = None + plans: List[Plan], scenario_priority_order: Optional[List[int]] = None ) -> Plan: - plan_priority_order = plan_priority_order or [] + 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: @@ -80,7 +79,7 @@ def choose_cheapest_relevant_plan( if (plan.record.cost_of_works or float("inf")) == min_cost ] - for priority_plan_id in plan_priority_order: + for priority_plan_id in scenario_priority_order: for plan in cheapest_plans: if plan.id == priority_plan_id: return plan diff --git a/backend/categorisation/tests/test_prioritised_plan_selected.py b/backend/categorisation/tests/test_prioritised_plan_selected.py index 5424dd5e..74eb8c69 100644 --- a/backend/categorisation/tests/test_prioritised_plan_selected.py +++ b/backend/categorisation/tests/test_prioritised_plan_selected.py @@ -39,7 +39,7 @@ def make_scenario(name: str, created_at: datetime, is_default: bool) -> Scenario multi_plan=False, is_default=is_default, ) - return Scenario(record=record, id=1 if is_default else 2) + return Scenario(record=record, id=3 if is_default else 4) def make_plan( @@ -54,17 +54,17 @@ def make_plan( ) -def test_prioritised_plan_selected(created_at_datetime: datetime) -> None: +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") - plan_priority_order: List[int] = [2, 1] + 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], - plan_priority_order=plan_priority_order, + scenario_priority_order=scenario_priority_order, ) # assert @@ -81,13 +81,13 @@ def test_cheapest_plan_returned_if_not_in_priority_list( minor_works_plan = make_plan( created_at_datetime, False, cost_of_works=100.0, name="EPC C - Minor Works" ) - plan_priority_order: List[int] = [1, 3] + 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], - plan_priority_order=plan_priority_order, + scenario_priority_order=scenario_priority_order, ) # assert From c1aa5716beed1fd9de3f4266918cf977c712e957 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 17:26:22 +0000 Subject: [PATCH 16/21] =?UTF-8?q?priority=20list=20is=20scenarios=20not=20?= =?UTF-8?q?plans=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/categorisation/processor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index fd0d9c89..ca58fb9d 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -79,9 +79,9 @@ def choose_cheapest_relevant_plan( if (plan.record.cost_of_works or float("inf")) == min_cost ] - for priority_plan_id in scenario_priority_order: + for priority_scenario_id in scenario_priority_order: for plan in cheapest_plans: - if plan.id == priority_plan_id: + if plan.scenario.id == priority_scenario_id: return plan return cheapest_plans[0] From 5ffa7290782ee5c2fb904b1c81453de6fc6074a8 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 18 Feb 2026 17:35:31 +0000 Subject: [PATCH 17/21] rename variable in invoke local lambda --- backend/categorisation/local_handler/invoke_local_lambda.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py index ce599ca9..a53e0d8e 100644 --- a/backend/categorisation/local_handler/invoke_local_lambda.py +++ b/backend/categorisation/local_handler/invoke_local_lambda.py @@ -11,7 +11,7 @@ payload = { { "portfolio_id": 556, "scenarios_to_consider": [1040, 1041], - "plan_priority_order": [], + "scenarios_priority_order": [], } ) } From b715a60d4a8f6e21aef7043385c44d805c810f2e Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Thu, 19 Feb 2026 09:25:55 +0000 Subject: [PATCH 18/21] typo in log --- backend/categorisation/processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index ca58fb9d..e1c0c6ff 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -165,4 +165,4 @@ def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None: logger.info("Successfully updated Plan default values") else: - logger.info("All plan default values already correct. Not udpating") + logger.info("All plan default values already correct. Not updating") From dae74d2f8b9ec93b7859a2bff4207055d374d295 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Thu, 19 Feb 2026 12:18:22 +0000 Subject: [PATCH 19/21] Fix bug where default plans were not being unset if they weren't included in scenarios to be considered --- .../db/functions/recommendations_functions.py | 21 ++++++++- .../local_handler/invoke_local_lambda.py | 2 +- backend/categorisation/processor.py | 46 ++++++++++++++----- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index 6f7dd41f..ff9b9dc8 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -639,6 +639,13 @@ def get_plans_by_scenario_ids(ids: List[int]) -> List[PlanModel]: return session_any.exec(stmt).scalars().all() +def get_plan_ids_by_scenario_ids(scenario_ids: List[int]) -> List[int]: + stmt = select(PlanModel.id).where(PlanModel.scenario_id.in_(scenario_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_scenarios_by_portfolio_id(portfolio_id: int) -> List[ScenarioModel]: stmt = select(ScenarioModel).where(ScenarioModel.portfolio_id == portfolio_id) with db_read_session() as session: @@ -657,7 +664,19 @@ def get_default_plan_ids_for_property(property_id: int) -> List[int]: # This should in reality always return exactly 1 ID, but there's currently # no database constraint to enforce that, so account for 0 or >1 stmt = select(PlanModel.id).where( - PlanModel.property_id == property_id and PlanModel.is_default + (PlanModel.property_id == property_id) & (PlanModel.is_default == True) + ) + 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_scenario_ids_for_portfolio(portfolio_id: int) -> List[int]: + # This should in reality always return exactly 1 ID, but there's currently + # no database constraint to enforce that, so account for 0 or >1 + stmt = select(ScenarioModel.id).where( + (ScenarioModel.portfolio_id == portfolio_id) + & (ScenarioModel.is_default == True) ) with db_read_session() as session: session_any: Any = session # Typehint as Any to satisfy Pylance... diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py index a53e0d8e..7d092d67 100644 --- a/backend/categorisation/local_handler/invoke_local_lambda.py +++ b/backend/categorisation/local_handler/invoke_local_lambda.py @@ -10,7 +10,7 @@ payload = { "body": json.dumps( { "portfolio_id": 556, - "scenarios_to_consider": [1040, 1041], + "scenarios_to_consider": [], "scenarios_priority_order": [], } ) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index e1c0c6ff..f619d5fd 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -3,7 +3,8 @@ from typing import Dict, List, Optional from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, - get_default_plan_ids_for_property, + get_default_scenario_ids_for_portfolio, + get_plan_ids_by_scenario_ids, get_plans_by_portfolio_id, get_plans_by_scenario_ids, get_scenarios_by_portfolio_id, @@ -24,6 +25,17 @@ def process_portfolio( ) -> None: logger.info(f"Processing portfolio {portfolio_id}") + if scenarios_to_consider: + if len(scenarios_to_consider) < 2: + raise ValueError( + "Cannot run auto categorisation for fewer than 2 scenarios" + ) + + if scenarios_to_consider is not None: + _unset_defaults_for_scenarios_not_being_considered( + portfolio_id, scenarios_to_consider + ) + plans: List[Plan] = _load_plans_for_portfolio(portfolio_id, scenarios_to_consider) plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans) @@ -37,11 +49,6 @@ def process_portfolio( property_plans, scenario_priority_order ) - # Unset existing default(s) in case they are outside the plans to consider - default_plan_ids: List[int] = get_default_plan_ids_for_property(property_id) - for id in default_plan_ids: - set_plan_and_scenario_default(id, False) - _update_default_flags( property_plans, cheapest_plan ) # TODO: we have already unset existing default(s), so this method can probably be a bit simpler now @@ -87,16 +94,33 @@ def choose_cheapest_relevant_plan( return cheapest_plans[0] +def _unset_defaults_for_scenarios_not_being_considered( + portfolio_id: int, scenarios_to_consider: List[int] +) -> None: + default_scenario_ids: List[int] = get_default_scenario_ids_for_portfolio( + portfolio_id + ) + scenarios_to_unset_default: List[int] = [] + + for id in default_scenario_ids: + if id not in scenarios_to_consider: + scenarios_to_unset_default.append(id) + + logger.info(f"Scenarios to unset defaults: {scenarios_to_unset_default}") + + if len(scenarios_to_unset_default) > 0: + plans_to_unset_default: List[int] = get_plan_ids_by_scenario_ids( + scenarios_to_unset_default + ) + for plan_id in plans_to_unset_default: + set_plan_and_scenario_default(plan_id, False) # TODO: do this in batch + + def _load_plans_for_portfolio( portfolio_id: int, scenarios_to_consider: Optional[List[int]] = None ) -> List[Plan]: if scenarios_to_consider: - if len(scenarios_to_consider) < 2: - raise ValueError( - "Cannot run auto categorisation for fewer than 2 scenarios" - ) - logger.info(f"Getting {len(scenarios_to_consider)} plans") plan_models: List[PlanModel] = get_plans_by_scenario_ids(scenarios_to_consider) From 9d12eef0e587537932a55fdb7726300380b4c60a Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Thu, 19 Feb 2026 12:22:19 +0000 Subject: [PATCH 20/21] Remove unused db functions --- .../db/functions/recommendations_functions.py | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/backend/app/db/functions/recommendations_functions.py b/backend/app/db/functions/recommendations_functions.py index ff9b9dc8..7ffcf603 100644 --- a/backend/app/db/functions/recommendations_functions.py +++ b/backend/app/db/functions/recommendations_functions.py @@ -625,13 +625,6 @@ def get_plans_by_portfolio_id(portfolio_id: int) -> List[PlanModel]: return session_any.exec(stmt).scalars().all() -def get_plans_by_ids(ids: List[int]) -> List[PlanModel]: - stmt = select(PlanModel).where(PlanModel.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_plans_by_scenario_ids(ids: List[int]) -> List[PlanModel]: stmt = select(PlanModel).where(PlanModel.scenario_id.in_(ids)) with db_read_session() as session: @@ -653,24 +646,6 @@ def get_scenarios_by_portfolio_id(portfolio_id: int) -> List[ScenarioModel]: return session_any.exec(stmt).scalars().all() -def get_scenario(scenario_id: int) -> Optional[ScenarioModel]: - stmt = select(ScenarioModel).where(ScenarioModel.id == scenario_id) - 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() - - -def get_default_plan_ids_for_property(property_id: int) -> List[int]: - # This should in reality always return exactly 1 ID, but there's currently - # no database constraint to enforce that, so account for 0 or >1 - stmt = select(PlanModel.id).where( - (PlanModel.property_id == property_id) & (PlanModel.is_default == True) - ) - 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_scenario_ids_for_portfolio(portfolio_id: int) -> List[int]: # This should in reality always return exactly 1 ID, but there's currently # no database constraint to enforce that, so account for 0 or >1 From 63bef436d086f791c10acbdb79371d585d5016ee Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Thu, 19 Feb 2026 12:44:41 +0000 Subject: [PATCH 21/21] do batch update per portfolio not per property --- .../local_handler/invoke_local_lambda.py | 2 +- backend/categorisation/processor.py | 45 +++++++++++-------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/backend/categorisation/local_handler/invoke_local_lambda.py b/backend/categorisation/local_handler/invoke_local_lambda.py index 7d092d67..127d2575 100644 --- a/backend/categorisation/local_handler/invoke_local_lambda.py +++ b/backend/categorisation/local_handler/invoke_local_lambda.py @@ -10,7 +10,7 @@ payload = { "body": json.dumps( { "portfolio_id": 556, - "scenarios_to_consider": [], + "scenarios_to_consider": [1039, 1041], "scenarios_priority_order": [], } ) diff --git a/backend/categorisation/processor.py b/backend/categorisation/processor.py index f619d5fd..966ecbf5 100644 --- a/backend/categorisation/processor.py +++ b/backend/categorisation/processor.py @@ -1,5 +1,5 @@ from collections import defaultdict -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Tuple from backend.app.db.functions.recommendations_functions import ( bulk_update_plans, @@ -40,6 +40,9 @@ def process_portfolio( plans_by_property: Dict[int, List[Plan]] = _group_plans_by_property(plans) + updated_plan_models: List[PlanModel] = [] + updated_scenario_models: List[ScenarioModel] = [] + for property_id, property_plans in plans_by_property.items(): if not property_plans: @@ -49,9 +52,16 @@ def process_portfolio( property_plans, scenario_priority_order ) - _update_default_flags( - property_plans, cheapest_plan - ) # TODO: we have already unset existing default(s), so this method can probably be a bit simpler now + updated_property_plan_models, updated_property_scenario_models = ( + _update_plan_and_scenario_objects(property_plans, cheapest_plan) + ) + + updated_plan_models.extend(updated_property_plan_models) + updated_scenario_models.extend(updated_property_scenario_models) + + if len(updated_plan_models) > 0: + bulk_update_plans(updated_plan_models, updated_scenario_models) + logger.info("Successfully updated Plan default values in database") def choose_cheapest_relevant_plan( @@ -106,7 +116,9 @@ def _unset_defaults_for_scenarios_not_being_considered( if id not in scenarios_to_consider: scenarios_to_unset_default.append(id) - logger.info(f"Scenarios to unset defaults: {scenarios_to_unset_default}") + logger.info( + f"Unsetting {scenarios_to_unset_default} as default scenario(s) as not included in provided list of scenarios to consider" + ) if len(scenarios_to_unset_default) > 0: plans_to_unset_default: List[int] = get_plan_ids_by_scenario_ids( @@ -164,7 +176,9 @@ def _group_plans_by_property(plans: List[Plan]) -> Dict[int, List[Plan]]: return grouped -def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None: +def _update_plan_and_scenario_objects( + plans: List[Plan], cheapest_plan: Plan +) -> Tuple[List[PlanModel], List[ScenarioModel]]: plans_to_update: List[Plan] = [] for plan in plans: @@ -176,17 +190,12 @@ def _update_default_flags(plans: List[Plan], cheapest_plan: Plan) -> None: plan.set_default(should_be_default) plans_to_update.append(plan) - if plans_to_update: - plan_models: List[PlanModel] = [] - scenario_models: List[ScenarioModel] = [] + plan_models: List[PlanModel] = [] + scenario_models: List[ScenarioModel] = [] - for plan in plans_to_update: - plan_model, scenario_model = plan.to_sqlalchemy() - plan_models.append(plan_model) - scenario_models.append(scenario_model) + for plan in plans_to_update: + plan_model, scenario_model = plan.to_sqlalchemy() + plan_models.append(plan_model) + scenario_models.append(scenario_model) - bulk_update_plans(plan_models, scenario_models) - logger.info("Successfully updated Plan default values") - - else: - logger.info("All plan default values already correct. Not updating") + return (plan_models, scenario_models)