From b79d16913e224ab6581f5029f1859a4bb25bd9a0 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 22 Jul 2026 09:35:57 +0000 Subject: [PATCH] plan_name column sources the source Scenario name, not plan.name `plan.name` is a freeform, user-entered label that is unset on modelling-generated Plans (0/205 populated on a real portfolio). The meaningful "name of the plan chosen" is the Scenario the home's default Plan came from, so `plan_name` now reads `scenario.name` via `plan.scenario_id` (LEFT JOIN scenario) on both export selections. On the default-plan export this labels each home with its source scenario (e.g. "Fabric Only" / "ASHP + PV"); on the scenario export it equals that sheet's scenario. Repository test seeds scenarios and asserts the source-scenario name; 62 tests pass, pyright --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/adr/0065-ara-scenario-export.md | 8 +++--- .../scenario_export_postgres_repository.py | 5 ++-- ...est_scenario_export_postgres_repository.py | 25 +++++++++++++------ 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/docs/adr/0065-ara-scenario-export.md b/docs/adr/0065-ara-scenario-export.md index a4e57c9eb..cf934ee46 100644 --- a/docs/adr/0065-ara-scenario-export.md +++ b/docs/adr/0065-ara-scenario-export.md @@ -230,6 +230,8 @@ the plan-selection `WHERE scenario_id = …` swapped for `WHERE is_default = TRU (no scenario parameter). A recipe without `plan_selection` (pre-existing tasks) defaults to `"scenario"`, so the change is backward-compatible. -**New column — `plan_name`.** Both selections now surface the chosen Plan's -`name` (from `plan.name`) as a `plan_name` column heading the plan post-works -block, so a reader can see which Plan produced each row. +**New column — `plan_name`.** Both selections surface the **source Scenario's +name** (`scenario.name`, via `plan.scenario_id`) as a `plan_name` column heading +the plan post-works block, so a reader sees which Scenario each home's Plan came +from (e.g. "Fabric Only" / "ASHP + PV"). The freeform `plan.name` is unset on +modelling-generated Plans, so the Scenario name is the meaningful identifier. diff --git a/repositories/scenario_export/scenario_export_postgres_repository.py b/repositories/scenario_export/scenario_export_postgres_repository.py index 20fac6bdf..03b1e91bb 100644 --- a/repositories/scenario_export/scenario_export_postgres_repository.py +++ b/repositories/scenario_export/scenario_export_postgres_repository.py @@ -29,18 +29,19 @@ _ROWS_SQL = text( "SELECT p.id, p.landlord_property_id, p.uprn, p.address, p.postcode," " np.post_sap_points, np.post_epc_rating, np.cost_of_works," " np.contingency_cost, np.co2_savings, np.energy_bill_savings," - " np.energy_consumption_savings, np.valuation_increase, np.plan_name" + " np.energy_consumption_savings, np.valuation_increase, sc.name AS plan_name" " FROM property p" " JOIN (" " SELECT DISTINCT ON (property_id) property_id, post_sap_points," " post_epc_rating, cost_of_works, contingency_cost, co2_savings," " energy_bill_savings, energy_consumption_savings, valuation_increase," - " name AS plan_name" + " scenario_id" " FROM plan" " WHERE portfolio_id = :portfolio_id AND scenario_id = :scenario_id" " AND property_id = ANY(:property_ids)" " ORDER BY property_id, created_at DESC, id DESC" " ) np ON np.property_id = p.id" + " LEFT JOIN scenario sc ON sc.id = np.scenario_id" " WHERE p.portfolio_id = :portfolio_id" " ORDER BY p.id" ) diff --git a/tests/repositories/scenario_export/test_scenario_export_postgres_repository.py b/tests/repositories/scenario_export/test_scenario_export_postgres_repository.py index 63d5744df..e0a4f825d 100644 --- a/tests/repositories/scenario_export/test_scenario_export_postgres_repository.py +++ b/tests/repositories/scenario_export/test_scenario_export_postgres_repository.py @@ -10,6 +10,7 @@ from sqlalchemy import Engine from sqlmodel import Session from datatypes.epc.domain.epc import Epc +from domain.modelling.portfolio_goal import PortfolioGoal from infrastructure.postgres.epc_property_table import ( # registers epc tables EpcBuildingPartModel, EpcEnergyElementModel, @@ -20,6 +21,7 @@ from infrastructure.postgres.epc_property_table import ( # registers epc tables from infrastructure.postgres.modelling import ( # registers `plan` + `recommendation` PlanModel, RecommendationModel, + ScenarioModel, ) from infrastructure.postgres.product_table import MaterialRow # registers `material` from infrastructure.postgres.property_override_table import ( # registers overrides @@ -671,8 +673,15 @@ def test_default_rows_for_selects_each_homes_default_plan_across_scenarios( # (property 1 → scenario 5, property 2 → scenario 9), plus a non-default Plan # on property 1 under a third scenario. The default-plan selection is # scenario-independent (ADR-0012 / ADR-0017): it must return exactly the two - # `is_default=True` Plans, carrying the Plan name, and ignore the non-default. + # `is_default=True` Plans, and `plan_name` carries the SOURCE SCENARIO's name + # (not the freeform `plan.name`, which is unset on modelling-generated Plans). with Session(db_engine) as session: + session.add(ScenarioModel(id=5, portfolio_id=100, name="Fabric Only", + goal=PortfolioGoal.INCREASING_EPC, goal_value="C")) + session.add(ScenarioModel(id=7, portfolio_id=100, name="Low Carbon", + goal=PortfolioGoal.INCREASING_EPC, goal_value="C")) + session.add(ScenarioModel(id=9, portfolio_id=100, name="ASHP + PV", + goal=PortfolioGoal.INCREASING_EPC, goal_value="C")) session.add( PropertyRow(id=1, portfolio_id=100, landlord_property_id="LP1", uprn=1) ) @@ -682,19 +691,19 @@ def test_default_rows_for_selects_each_homes_default_plan_across_scenarios( session.add( PlanModel( portfolio_id=100, property_id=1, scenario_id=5, is_default=True, - name="Fabric First", post_sap_points=78.0, + post_sap_points=78.0, ) ) session.add( PlanModel( portfolio_id=100, property_id=1, scenario_id=7, is_default=False, - name="Low Carbon", post_sap_points=81.0, + post_sap_points=81.0, ) ) session.add( PlanModel( portfolio_id=100, property_id=2, scenario_id=9, is_default=True, - name="Whole House", post_sap_points=69.0, + post_sap_points=69.0, ) ) session.commit() @@ -705,11 +714,11 @@ def test_default_rows_for_selects_each_homes_default_plan_across_scenarios( portfolio_id=100, property_ids=[1, 2] ) - # assert — one row per home, the default Plan, with its name; the non-default - # "Low Carbon" (81.0) never appears. + # assert — one row per home, the default Plan, with its source scenario name; + # the non-default plan under "Low Carbon" (81.0) never appears. by_property = {r.property_id: r for r in rows} assert set(by_property) == {1, 2} - assert by_property[1].plan_name == "Fabric First" + assert by_property[1].plan_name == "Fabric Only" assert by_property[1].post_sap_points == 78.0 - assert by_property[2].plan_name == "Whole House" + assert by_property[2].plan_name == "ASHP + PV" assert by_property[2].post_sap_points == 69.0