From 72f3e3a72de69a008a776a0e34912f402610901f Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 22 Jul 2026 09:21:38 +0000 Subject: [PATCH] Add default-plan export selection + plan_name column (ADR-0065) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second export selection alongside the per-Scenario one, chosen by `plan_selection` on the export request: - "scenario" (default, unchanged): one sheet per Scenario, freshest Plan per (property, scenario); scenario_ids required. - "default" (new): a single "Default Plans" sheet of each home's is_default Plan (one-per-property across all Scenarios, ADR-0012/0017) — the portfolio's current state, one row per home; scenario_ids ignored. Repository gains `default_rows_for(portfolio_id, property_ids)`: same read-model and Effective-EPC join as `rows_for`, with the plan-selection WHERE swapped from `scenario_id = :scenario_id` to `is_default = TRUE`. Orchestrator branches on plan_selection; the router validates it and only requires scenario_ids for the scenario selection. A recipe without plan_selection defaults to "scenario", so the change is backward-compatible. Both selections now surface the chosen Plan's `name` as a `plan_name` column. Tests: repository default_rows_for (scenario-independent selection, plan_name, non-default excluded); orchestrator default path (one "Default Plans" sheet; empty selection is a recorded failure). 62 passed, pyright --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- applications/ara_export/handler.py | 9 ++- backend/app/exports/router.py | 22 +++++- docs/adr/0065-ara-scenario-export.md | 25 +++++++ domain/scenario_export/scenario_sheet.py | 3 + orchestration/scenario_export_orchestrator.py | 65 ++++++++++++++--- .../scenario_export_postgres_repository.py | 51 +++++++++++-- .../scenario_export_repository.py | 14 ++++ .../test_scenario_export_orchestrator.py | 73 ++++++++++++++++++- ...est_scenario_export_postgres_repository.py | 51 +++++++++++++ 9 files changed, 289 insertions(+), 24 deletions(-) diff --git a/applications/ara_export/handler.py b/applications/ara_export/handler.py index 230441683..b94170caf 100644 --- a/applications/ara_export/handler.py +++ b/applications/ara_export/handler.py @@ -66,10 +66,14 @@ def handler(body: dict[str, Any], context: Any) -> dict[str, Any]: property_ids: list[int] = [int(x) for x in recipe["property_ids"]] recipient_email = str(recipe["recipient_email"]) export_name = str(recipe["export_name"]) + # ADR-0065 plan selection: "scenario" (default) or "default" (each + # home's is_default Plan). Absent on pre-existing recipes → "scenario". + plan_selection = str(recipe.get("plan_selection") or "scenario") logger.info( - "ara_export: starting export for %d properties across %d scenario(s) " - "(subtask=%s)", + "ara_export: starting %s export for %d properties across %d " + "scenario(s) (subtask=%s)", + plan_selection, len(property_ids), len(scenario_ids), trigger.subtask_id, @@ -89,6 +93,7 @@ def handler(body: dict[str, Any], context: Any) -> dict[str, Any]: property_ids=property_ids, recipient_email=recipient_email, export_name=export_name, + plan_selection=plan_selection, ) finally: session.close() diff --git a/backend/app/exports/router.py b/backend/app/exports/router.py index e4d99b9d0..a9ec9afdd 100644 --- a/backend/app/exports/router.py +++ b/backend/app/exports/router.py @@ -92,15 +92,32 @@ async def scenario_export( raw_scenario_ids: Any = config.get("scenario_ids") or [] raw_property_ids: Any = config.get("property_ids") or [] raw_filters: Any = config.get("filters") or {} + # ADR-0065 plan selection: "scenario" (per-Scenario sheets, the default) or + # "default" (one sheet of each home's is_default Plan). Absent → "scenario". + plan_selection: Any = config.get("plan_selection") or "scenario" if not isinstance(portfolio_id, int): raise HTTPException( status_code=400, detail="task.inputs must provide an integer portfolio_id." ) - if not _is_int_list(raw_scenario_ids) or not raw_scenario_ids: + if plan_selection not in ("scenario", "default"): raise HTTPException( status_code=400, - detail="task.inputs scenario_ids must be a non-empty list of integers.", + detail='task.inputs plan_selection must be "scenario" or "default".', + ) + # scenario_ids is required only for a per-Scenario export; the default-plan + # selection is scenario-independent (one default Plan per home). + if plan_selection == "scenario": + if not _is_int_list(raw_scenario_ids) or not raw_scenario_ids: + raise HTTPException( + status_code=400, + detail="task.inputs scenario_ids must be a non-empty list of " + "integers for a scenario export.", + ) + elif raw_scenario_ids and not _is_int_list(raw_scenario_ids): + raise HTTPException( + status_code=400, + detail="task.inputs scenario_ids, if provided, must be integers.", ) if not _is_int_list(raw_property_ids): raise HTTPException( @@ -145,6 +162,7 @@ async def scenario_export( "property_ids": resolved_property_ids, "recipient_email": recipient_email, "export_name": f"portfolio-{portfolio_id}-{body.task_id}", + "plan_selection": plan_selection, } created = tasks.create_export_subtask(body.task_id, subtask_id, recipe) if not created: diff --git a/docs/adr/0065-ara-scenario-export.md b/docs/adr/0065-ara-scenario-export.md index 7cebda333..a4e57c9eb 100644 --- a/docs/adr/0065-ara-scenario-export.md +++ b/docs/adr/0065-ara-scenario-export.md @@ -208,3 +208,28 @@ presigned URL + email (ADR-0059).** the intended shareability, made explicit. - `scripts/data_exports.py`'s query/column logic is de-duplicated into the new modules; the standalone CLI either wraps them or is retired. + +## Addendum — plan-selection option (default plan per home) + +A second **plan selection** was added alongside the original per-Scenario export, +chosen by `plan_selection` on the export request: + +- **`"scenario"` (default, unchanged):** one sheet per requested Scenario, the + freshest Plan per `(property, scenario)`. `scenario_ids` required. +- **`"default"` (new):** a single sheet titled **"Default Plans"** of each home's + **default Plan** (`plan.is_default = TRUE`), which is one-per-property across + all Scenarios (ADR-0012 / ADR-0017) — the portfolio's current state, one row + per home. `scenario_ids` is not required and is ignored. + +The router (`backend/app/exports/router.py`) validates `plan_selection ∈ +{"scenario", "default"}` and only requires a non-empty `scenario_ids` for the +scenario selection; the recipe carries `plan_selection` through to the handler +and orchestrator. The repository gains `default_rows_for(portfolio_id, +property_ids)` — the same read-model and Effective-EPC join as `rows_for`, with +the plan-selection `WHERE scenario_id = …` swapped for `WHERE is_default = TRUE` +(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. diff --git a/domain/scenario_export/scenario_sheet.py b/domain/scenario_export/scenario_sheet.py index ae3860d23..055c91812 100644 --- a/domain/scenario_export/scenario_sheet.py +++ b/domain/scenario_export/scenario_sheet.py @@ -91,6 +91,7 @@ _TOTAL_COLUMNS: tuple[str, ...] = ("total_retrofit_cost",) # The default Plan's post-works figures, taken straight from the persisted Plan # (the SAP calculator's output) with no recompute (ADR-0065). _PLAN_COLUMNS: tuple[str, ...] = ( + "plan_name", "post_sap_points", "post_epc_rating", "cost_of_works", @@ -164,6 +165,7 @@ class PropertyScenarioData: is_expired: Optional[bool] = None current_epc_rating: Optional[str] = None current_sap_points: Optional[float] = None + plan_name: Optional[str] = None post_sap_points: Optional[float] = None post_epc_rating: Optional[str] = None cost_of_works: Optional[float] = None @@ -231,6 +233,7 @@ def _shape_row(prop: PropertyScenarioData) -> dict[str, Any]: row["energy_cost_savings"] = _sum([m.energy_cost_savings for m in prop.measures]) row["total_retrofit_cost"] = _sum([m.estimated_cost for m in prop.measures]) # The default Plan's post-works figures, passed through as persisted. + row["plan_name"] = _blank(prop.plan_name) row["post_sap_points"] = _blank(prop.post_sap_points) row["post_epc_rating"] = _blank(prop.post_epc_rating) row["cost_of_works"] = _blank(prop.cost_of_works) diff --git a/orchestration/scenario_export_orchestrator.py b/orchestration/scenario_export_orchestrator.py index 39d25eccb..657d15b7a 100644 --- a/orchestration/scenario_export_orchestrator.py +++ b/orchestration/scenario_export_orchestrator.py @@ -29,6 +29,9 @@ from repositories.email.email_sender import EmailSender logger = logging.getLogger(__name__) +# The single sheet title for the default-plan selection (ADR-0065). +_DEFAULT_PLAN_SHEET_NAME = "Default Plans" + class ExportRowReader(Protocol): def rows_for( @@ -39,6 +42,13 @@ class ExportRowReader(Protocol): property_ids: Sequence[int], ) -> list[PropertyScenarioData]: ... + def default_rows_for( + self, + *, + portfolio_id: int, + property_ids: Sequence[int], + ) -> list[PropertyScenarioData]: ... + class ScenarioNames(Protocol): def name_for(self, scenario_id: int) -> str: ... @@ -72,6 +82,33 @@ class ScenarioExportOrchestrator: self._url_ttl_seconds = url_ttl_seconds self._key_prefix = key_prefix + def _scenario_sheets( + self, + portfolio_id: int, + scenario_ids: Sequence[int], + property_ids: Sequence[int], + ) -> list[tuple[str, ExportSheet]]: + named_sheets: list[tuple[str, ExportSheet]] = [] + for scenario_id in scenario_ids: + rows = self._rows.rows_for( + portfolio_id=portfolio_id, + scenario_id=scenario_id, + property_ids=property_ids, + ) + named_sheets.append( + (self._scenarios.name_for(scenario_id), shape_scenario_sheet(rows)) + ) + return named_sheets + + def _default_plan_sheets( + self, portfolio_id: int, property_ids: Sequence[int] + ) -> list[tuple[str, ExportSheet]]: + # One sheet: each home's default Plan (`is_default`), across Scenarios. + rows = self._rows.default_rows_for( + portfolio_id=portfolio_id, property_ids=property_ids + ) + return [(_DEFAULT_PLAN_SHEET_NAME, shape_scenario_sheet(rows))] + def run( self, *, @@ -80,26 +117,30 @@ class ScenarioExportOrchestrator: property_ids: Sequence[int], recipient_email: str, export_name: str, + plan_selection: str = "scenario", ) -> ScenarioExportResult: - named_sheets: list[tuple[str, ExportSheet]] = [] - row_count = 0 - for scenario_id in scenario_ids: - rows = self._rows.rows_for( - portfolio_id=portfolio_id, - scenario_id=scenario_id, - property_ids=property_ids, + # ADR-0065 two selections: + # - "scenario": one sheet per requested Scenario, the freshest Plan per + # (property, scenario). + # - "default": ONE sheet of each home's default Plan (`is_default`), + # one-per-property across all Scenarios — the portfolio's current + # state. `scenario_ids` is ignored. + if plan_selection == "default": + named_sheets = self._default_plan_sheets(portfolio_id, property_ids) + else: + named_sheets = self._scenario_sheets( + portfolio_id, scenario_ids, property_ids ) - sheet = shape_scenario_sheet(rows) - named_sheets.append((self._scenarios.name_for(scenario_id), sheet)) - row_count += len(sheet.rows) + row_count = sum(len(sheet.rows) for _, sheet in named_sheets) if row_count == 0: - # No Property had a default Plan under any requested Scenario (model - # A) — a recorded failure, never an empty workbook emailed (ADR-0065). + # No Property had a Plan under the selection (model A) — a recorded + # failure, never an empty workbook emailed (ADR-0065). raise SubTaskFailure( "the scenario export selection produced no exportable rows", details={ "portfolio_id": portfolio_id, + "plan_selection": plan_selection, "scenario_ids": list(scenario_ids), "property_count": len(property_ids), }, diff --git a/repositories/scenario_export/scenario_export_postgres_repository.py b/repositories/scenario_export/scenario_export_postgres_repository.py index c17c220d9..20fac6bdf 100644 --- a/repositories/scenario_export/scenario_export_postgres_repository.py +++ b/repositories/scenario_export/scenario_export_postgres_repository.py @@ -29,12 +29,13 @@ _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.energy_consumption_savings, np.valuation_increase, np.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" + " energy_bill_savings, energy_consumption_savings, valuation_increase," + " name AS plan_name" " FROM plan" " WHERE portfolio_id = :portfolio_id AND scenario_id = :scenario_id" " AND property_id = ANY(:property_ids)" @@ -65,6 +66,21 @@ _MEASURES_SQL = text( " AND r.already_installed = FALSE" ) +# DEFAULT-PLAN variants (ADR-0065 "default plan per home" selection). Each home's +# ONE default Plan (`is_default = TRUE`), which is one-per-property across all +# Scenarios (ADR-0012 / ADR-0017) — so there is no `scenario_id` filter. The +# `DISTINCT ON (property_id) … created_at DESC` guard is retained as belt-and- +# braces against a transient double-default during a re-baseline write; steady +# state has exactly one. Otherwise identical to the scenario SQL above. +_DEFAULT_ROWS_SQL = text( + str(_ROWS_SQL) + .replace("scenario_id = :scenario_id", "is_default = TRUE") +) +_DEFAULT_MEASURES_SQL = text( + str(_MEASURES_SQL) + .replace("scenario_id = :scenario_id", "is_default = TRUE") +) + # The Effective-EPC descriptive picture (ADR-0065). Header facts and current # performance are matched by UPRN (the latest epc_property for the source), # because the PasHub re-fetch lands records with property_id = NULL — a @@ -221,9 +237,31 @@ class ScenarioExportPostgresRepository(ScenarioExportRepository): "scenario_id": scenario_id, "property_ids": list(property_ids), } - measures = self._measures_by_property(params) + return self._assemble_rows(_ROWS_SQL, _MEASURES_SQL, params) + + def default_rows_for( + self, + *, + portfolio_id: int, + property_ids: Sequence[int], + ) -> list[PropertyScenarioData]: + if not property_ids: + return [] + # No scenario_id: the default-plan SQL selects on `is_default = TRUE`. + params: dict[str, Any] = { + "portfolio_id": portfolio_id, + "property_ids": list(property_ids), + } + return self._assemble_rows( + _DEFAULT_ROWS_SQL, _DEFAULT_MEASURES_SQL, params + ) + + def _assemble_rows( + self, rows_sql: Any, measures_sql: Any, params: dict[str, Any] + ) -> list[PropertyScenarioData]: + measures = self._measures_by_property(params, measures_sql) epc = self._effective_epc_by_property(params) - result = self._session.connection().execute(_ROWS_SQL, params) + result = self._session.connection().execute(rows_sql, params) rows: list[PropertyScenarioData] = [] for row in result.all(): facts = epc.get(row[0], {}) @@ -242,6 +280,7 @@ class ScenarioExportPostgresRepository(ScenarioExportRepository): energy_bill_savings=_float(row[10]), energy_consumption_savings=_float(row[11]), valuation_increase=_float(row[12]), + plan_name=_str(row[13]), property_type=facts.get("property_type"), built_form=facts.get("built_form"), property_age_band=facts.get("property_age_band"), @@ -264,9 +303,9 @@ class ScenarioExportPostgresRepository(ScenarioExportRepository): return rows def _measures_by_property( - self, params: dict[str, Any] + self, params: dict[str, Any], measures_sql: Any = _MEASURES_SQL ) -> dict[int, list[ExportMeasure]]: - result = self._session.connection().execute(_MEASURES_SQL, params) + result = self._session.connection().execute(measures_sql, params) by_property: dict[int, list[ExportMeasure]] = {} for row in result.all(): by_property.setdefault(row[0], []).append( diff --git a/repositories/scenario_export/scenario_export_repository.py b/repositories/scenario_export/scenario_export_repository.py index dfc9cb113..a3cae4e7d 100644 --- a/repositories/scenario_export/scenario_export_repository.py +++ b/repositories/scenario_export/scenario_export_repository.py @@ -23,3 +23,17 @@ class ScenarioExportRepository(ABC): selected measures. A Property with no default Plan for the Scenario is absent (scenario-scoped, model A).""" ... + + @abstractmethod + def default_rows_for( + self, + *, + portfolio_id: int, + property_ids: Sequence[int], + ) -> list[PropertyScenarioData]: + """The export rows for the Properties in ``property_ids`` selected by + each home's **default Plan** (``plan.is_default = TRUE``), which is + one-per-property across all Scenarios (ADR-0012 / ADR-0017). Unlike + ``rows_for`` this is NOT scenario-scoped — it is the portfolio's current + state, one row per home. A Property with no default Plan is absent.""" + ... diff --git a/tests/orchestration/test_scenario_export_orchestrator.py b/tests/orchestration/test_scenario_export_orchestrator.py index 86ce864ea..26c678da3 100644 --- a/tests/orchestration/test_scenario_export_orchestrator.py +++ b/tests/orchestration/test_scenario_export_orchestrator.py @@ -20,10 +20,15 @@ EXPORTS_BUCKET = "domna-exports-dev" class _FakeRows: - """Returns preset export rows per scenario id.""" + """Returns preset export rows per scenario id, and a preset default-plan set.""" - def __init__(self, by_scenario: dict[int, list[PropertyScenarioData]]) -> None: + def __init__( + self, + by_scenario: dict[int, list[PropertyScenarioData]], + default_rows: Optional[list[PropertyScenarioData]] = None, + ) -> None: self._by_scenario = by_scenario + self._default_rows = default_rows or [] def rows_for( self, @@ -34,6 +39,14 @@ class _FakeRows: ) -> list[PropertyScenarioData]: return self._by_scenario.get(scenario_id, []) + def default_rows_for( + self, + *, + portfolio_id: int, + property_ids: Sequence[int], + ) -> list[PropertyScenarioData]: + return self._default_rows + class _FakeScenarioNames: def __init__(self, names: dict[int, str]) -> None: @@ -182,3 +195,59 @@ def test_a_selection_that_yields_no_rows_is_a_recorded_failure( export_name="portfolio-100", ) assert email.sent == [] + + +def test_default_plan_selection_builds_one_sheet_from_each_homes_default_plan( + exports: S3Client, +) -> None: + # arrange — plan_selection="default" ignores scenario_ids and reads each + # home's default Plan (ADR-0065). The fake returns two default rows and + # would raise if `rows_for` (the scenario path) were called instead. + orchestrator = ScenarioExportOrchestrator( + rows=_FakeRows({}, default_rows=[_row(1), _row(2)]), + scenarios=_FakeScenarioNames({}), + exports=exports, + email=_RecordingEmail(), + url_ttl_seconds=3600, + ) + + # act — no scenario_ids needed for the default-plan selection. + result = orchestrator.run( + portfolio_id=100, + scenario_ids=[], + property_ids=[1, 2], + recipient_email="user@example.com", + export_name="portfolio-100-default", + plan_selection="default", + ) + + # assert — exactly one sheet, titled "Default Plans", with both homes' rows. + assert result.sheet_count == 1 + assert result.row_count == 2 + workbook = load_workbook(BytesIO(exports.get_object(result.export_s3_key))) + assert workbook.sheetnames == ["Default Plans"] + + +def test_default_selection_with_no_default_plans_is_a_recorded_failure( + exports: S3Client, +) -> None: + # arrange — no home has a default Plan. + orchestrator = ScenarioExportOrchestrator( + rows=_FakeRows({}, default_rows=[]), + scenarios=_FakeScenarioNames({}), + exports=exports, + email=_RecordingEmail(), + url_ttl_seconds=3600, + ) + + # act / assert — a selection that yields no rows is a failure, never an + # empty workbook emailed. + with pytest.raises(SubTaskFailure): + orchestrator.run( + portfolio_id=100, + scenario_ids=[], + property_ids=[1, 2], + recipient_email="user@example.com", + export_name="portfolio-100-default", + plan_selection="default", + ) 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 0210502ad..63d5744df 100644 --- a/tests/repositories/scenario_export/test_scenario_export_postgres_repository.py +++ b/tests/repositories/scenario_export/test_scenario_export_postgres_repository.py @@ -662,3 +662,54 @@ def test_falls_back_to_the_predicted_epc_when_there_is_no_lodged_epc( row = rows[0] assert row.property_type == "Bungalow" assert row.walls == "Predicted solid brick wall" + + +def test_default_rows_for_selects_each_homes_default_plan_across_scenarios( + db_engine: Engine, +) -> None: + # arrange — two homes, each with its default Plan under a DIFFERENT scenario + # (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. + with Session(db_engine) as session: + session.add( + PropertyRow(id=1, portfolio_id=100, landlord_property_id="LP1", uprn=1) + ) + session.add( + PropertyRow(id=2, portfolio_id=100, landlord_property_id="LP2", uprn=2) + ) + session.add( + PlanModel( + portfolio_id=100, property_id=1, scenario_id=5, is_default=True, + name="Fabric First", 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, + ) + ) + session.add( + PlanModel( + portfolio_id=100, property_id=2, scenario_id=9, is_default=True, + name="Whole House", post_sap_points=69.0, + ) + ) + session.commit() + + # act — no scenario_id. + with Session(db_engine) as session: + rows = ScenarioExportPostgresRepository(session).default_rows_for( + 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. + 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].post_sap_points == 78.0 + assert by_property[2].plan_name == "Whole House" + assert by_property[2].post_sap_points == 69.0