From 71a86f87fc33368a93debb49e4a9d90f9bfe7876 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:03:28 +0000 Subject: [PATCH 01/56] ADR-0055 + ADR-0056: Modelling Run distributor decisions and glossary The distributor pre-creates batch sub_tasks under the app-owned task and sub_task status is a record, not retry fuel; filter resolution is a shared contract with the app's preview. Scenario immutability replaces the Scenario Snapshot fiction; Modelling Run, Distributor and First Run terms pinned in CONTEXT.md. Co-Authored-By: Claude Fable 5 --- CONTEXT.md | 20 ++-- ...un-batches-attach-to-the-app-owned-task.md | 103 ++++++++++++++++++ ...-a-shared-contract-with-the-app-preview.md | 59 ++++++++++ 3 files changed, 174 insertions(+), 8 deletions(-) create mode 100644 docs/adr/0055-modelling-run-batches-attach-to-the-app-owned-task.md create mode 100644 docs/adr/0056-modelling-run-filter-resolution-is-a-shared-contract-with-the-app-preview.md diff --git a/CONTEXT.md b/CONTEXT.md index 81b2716c3..ee7f43d21 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -199,8 +199,16 @@ _Avoid_: rebaseline (that is a specific ML trigger — see Rebaselining), enrich The third stage. Takes the baselined Property plus a set of **Scenarios** and produces **Recommendations** → an **Optimised Package** → **Plans**, persisted to repos. A separate orchestrator from Baseline so the single-property flow can stop after Baseline and only run Modelling when the user hits "play". _Avoid_: scoring (overloaded), recommendation engine +**Modelling Run**: +One triggered unit of modelling over a portfolio: a target set of **Properties** resolved from user-chosen filters (no filters = the whole portfolio) crossed with one or more **Scenarios**, producing one **Plan** per (Scenario, Property). Tracked as a single task; its batch sub_tasks are the units of execution, of failure, and of re-run. Re-runs append Plans; readers take the latest per Property. +_Avoid_: modelling job (ambiguous with one lambda invocation), batch (that is one message-worth of a run), trigger run + +**Distributor**: +The role of the Modelling Run entry point: validate the request, resolve the filters to a concrete Property set, pre-create the run's batch sub_tasks, and fan the work out to the modelling workers. It never models synchronously and owns nothing after the fan-out — progress and terminal state roll up from the workers' sub_task statuses. +_Avoid_: trigger endpoint (names the URL, not the role), orchestrator (taken — stage orchestrators, TaskOrchestrator) + **First Run**: -The use case where a Property has only a row in the property table (post address→UPRN matching) and no existing **Plan**: the pipeline runs Ingestion → Baseline → Modelling end-to-end over a batch. The first sibling lambda being built (`ara_first_run`). +The special case of a **Modelling Run** where a Property has only a row in the property table (post address→UPRN matching) and no existing **Plan**: the pipeline runs Ingestion → Baseline → Modelling end-to-end. Executed by the same `modelling_e2e` worker as re-runs — the lambda originally planned as `ara_first_run` serves both. _Avoid_: initial run, cold run ### ML training @@ -238,12 +246,8 @@ _Avoid_: emission factors (ambiguous), CO2 rates ### Outputs **Scenario**: -A named portfolio-level retrofit plan, built by a user in the scenario-builder UI and persisted before any modelling fires; carries the overall goal (e.g. Increasing EPC), budget, exclusions, housing type, and the set of measure types it permits. The model is triggered against one or more Scenarios at once; each Scenario yields one Plan per Property. -_Avoid_: project, batch, run-set - -**Scenario Snapshot**: -A frozen copy of a Scenario pinned at trigger time, keyed by (task, scenario); used by the modelling pipeline so mid-run edits to the live Scenario do not affect an in-flight job. Snapshots are read-only and may be garbage-collected after the task completes. -_Avoid_: scenario version, frozen scenario, pinned scenario +A named portfolio-level retrofit plan, built by a user in the scenario-builder UI and persisted before any modelling fires; carries the overall goal (e.g. Increasing EPC), budget, exclusions, housing type, and the set of measure types it permits. The model is triggered against one or more Scenarios at once; each Scenario yields one Plan per Property. Scenarios are **immutable after creation**: they may be renamed, and deleted only while no Plans reference them — their modelling-relevant values never change, so an in-flight run can safely read the live row (no snapshot/pinning machinery is needed or exists). +_Avoid_: project, batch, run-set, scenario snapshot (described pinning machinery that immutability makes unnecessary; removed 2026-07) **Plan**: The per-Property output of one Scenario's modelling run; carries the **Optimised Package** selected for the Property (its **Plan Measures**) and the Property's post-retrofit figures (SAP / kWh / CO₂ / bills). A Property modelled against N Scenarios in one trigger ends up with N Plans. @@ -443,7 +447,7 @@ addresses ``, ` Date: Tue, 7 Jul 2026 11:03:28 +0000 Subject: [PATCH 02/56] =?UTF-8?q?A=20run=20with=20finished=20and=20queued?= =?UTF-8?q?=20batches=20rolls=20up=20in=20progress,=20not=20waiting=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- tests/domain/tasks/test_tasks.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/domain/tasks/test_tasks.py b/tests/domain/tasks/test_tasks.py index ba82412b1..ff30a3671 100644 --- a/tests/domain/tasks/test_tasks.py +++ b/tests/domain/tasks/test_tasks.py @@ -105,6 +105,18 @@ def test_recalculate_any_in_progress_marks_in_progress() -> None: assert t.job_completed is None +def test_recalculate_complete_and_waiting_mix_marks_in_progress() -> None: + # arrange + t = Task.create(task_source="manual:test") + + # act + t.recalculate_from_subtasks([SubTaskStatus.COMPLETE, SubTaskStatus.WAITING]) + + # assert + assert t.status is TaskStatus.IN_PROGRESS + assert t.job_completed is None + + def test_recalculate_all_complete_marks_complete() -> None: # arrange t = Task.create(task_source="manual:test") From 5b607171338f31d0d4985c5a320875e93508554e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:04:50 +0000 Subject: [PATCH 03/56] =?UTF-8?q?A=20run=20with=20finished=20and=20queued?= =?UTF-8?q?=20batches=20rolls=20up=20in=20progress,=20not=20waiting=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- domain/tasks/tasks.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/domain/tasks/tasks.py b/domain/tasks/tasks.py index ead253ab6..992ae8d71 100644 --- a/domain/tasks/tasks.py +++ b/domain/tasks/tasks.py @@ -70,11 +70,12 @@ class Task: def recalculate_from_subtasks(self, statuses: list[SubTaskStatus]) -> None: """Recompute Task.status from its SubTasks' statuses. - Rule (preserved from legacy _update_task_progress): - - any FAILED → FAILED - - all COMPLETE → COMPLETE - - any IN_PROGRESS → IN_PROGRESS - - otherwise → WAITING + Rule: + - any FAILED → FAILED + - all COMPLETE → COMPLETE + - any IN_PROGRESS or COMPLETE → IN_PROGRESS (finished batches plus + queued batches is a run in progress, not a waiting one — ADR-0055) + - all WAITING → WAITING Empty list is a no-op (newly-created task with no subtasks). """ @@ -87,7 +88,10 @@ class Task: elif all(s is SubTaskStatus.COMPLETE for s in statuses): self.status = TaskStatus.COMPLETE self.job_completed = now - elif SubTaskStatus.IN_PROGRESS in statuses: + elif ( + SubTaskStatus.IN_PROGRESS in statuses + or SubTaskStatus.COMPLETE in statuses + ): self.status = TaskStatus.IN_PROGRESS self.job_completed = None else: From 3d643965b5a9c73f05177e7adfbb76e9ff009850 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:05:34 +0000 Subject: [PATCH 04/56] =?UTF-8?q?A=20failed=20batch=20sub=5Ftask=20restart?= =?UTF-8?q?s=20for=20a=20re-run=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- tests/domain/tasks/test_subtasks.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/domain/tasks/test_subtasks.py b/tests/domain/tasks/test_subtasks.py index 8cee44960..d379439c6 100644 --- a/tests/domain/tasks/test_subtasks.py +++ b/tests/domain/tasks/test_subtasks.py @@ -58,6 +58,20 @@ def test_start_rejects_from_terminal_status() -> None: st.start() +def test_start_restarts_a_failed_subtask_for_a_rerun() -> None: + # arrange + st = SubTask.create(task_id=uuid4()) + st.start() + st.fail(RuntimeError("degenerate prediction")) + + # act + st.start() + + # assert + assert st.status is SubTaskStatus.IN_PROGRESS + assert st.job_completed is None + + def test_complete_marks_outputs_and_job_completed() -> None: # arrange st = SubTask.create(task_id=uuid4()) From 30e30541461be29d1f648af4822db848ef863756 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:08:47 +0000 Subject: [PATCH 05/56] =?UTF-8?q?A=20failed=20batch=20sub=5Ftask=20restart?= =?UTF-8?q?s=20for=20a=20re-run=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- domain/tasks/subtasks.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/domain/tasks/subtasks.py b/domain/tasks/subtasks.py index bd49a6ec6..95f9ec5bc 100644 --- a/domain/tasks/subtasks.py +++ b/domain/tasks/subtasks.py @@ -35,11 +35,14 @@ class SubTask: ) def start(self, cloud_logs_url: Optional[str] = None) -> None: - if self.status not in (SubTaskStatus.WAITING, SubTaskStatus.IN_PROGRESS): + # FAILED may restart: a failed batch is re-run by re-sending its own + # inputs, and its completion un-fails the parent Task (ADR-0055). + if self.status is SubTaskStatus.COMPLETE: raise ValueError(f"cannot start subtask in status {self.status}") if self.job_started is None: self.job_started = datetime.now(timezone.utc) self.status = SubTaskStatus.IN_PROGRESS + self.job_completed = None if cloud_logs_url is not None: self.cloud_logs_url = cloud_logs_url From d0439dd11e7ba9cfc327d03c315c8020c3cf0a0c Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:10:43 +0000 Subject: [PATCH 06/56] =?UTF-8?q?Completing=20a=20re-run=20failed=20batch?= =?UTF-8?q?=20un-fails=20the=20task=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- tests/orchestration/test_task_orchestrator.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/orchestration/test_task_orchestrator.py b/tests/orchestration/test_task_orchestrator.py index 8767c8c15..dc822e00c 100644 --- a/tests/orchestration/test_task_orchestrator.py +++ b/tests/orchestration/test_task_orchestrator.py @@ -263,6 +263,29 @@ def test_run_subtasks_isolates_a_failing_item_and_continues( assert harness.tasks.get(task.id).status is TaskStatus.FAILED +def test_completing_a_rerun_failed_subtask_unfails_the_task( + harness: Harness, +) -> None: + """A failed batch is re-run by re-sending its own inputs; when the re-run + completes, the parent Task recomputes to COMPLETE (ADR-0055).""" + # arrange — two children: one failed (task FAILED), the other complete + task, failed_batch = harness.orchestrator.create_task_with_subtask( + task_source="manual:test" + ) + healthy_batch = harness.orchestrator.create_child_subtask(task.id) + harness.orchestrator.complete_subtask(healthy_batch.id) + harness.orchestrator.fail_subtask(failed_batch.id, RuntimeError("boom")) + assert harness.tasks.get(task.id).status is TaskStatus.FAILED + + # act — the re-run of the failed batch + harness.orchestrator.run_subtask(failed_batch.id, work=lambda: "fixed") + + # assert — no failed children remain, so the task un-fails to COMPLETE + task_after = harness.tasks.get(task.id) + assert task_after.status is TaskStatus.COMPLETE + assert task_after.job_completed is not None + + def test_cascade_short_circuits_once_task_already_failed(harness: Harness) -> None: """Once the Task is FAILED, completing another SubTask leaves it FAILED — the terminal state is not recomputed away.""" From 9cbb8a95b5b9c53ef3a4e04f7e1d8c8920f5050e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:11:52 +0000 Subject: [PATCH 07/56] =?UTF-8?q?Completing=20a=20re-run=20failed=20batch?= =?UTF-8?q?=20un-fails=20the=20task=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- orchestration/task_orchestrator.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/orchestration/task_orchestrator.py b/orchestration/task_orchestrator.py index 15915d9f1..58cb22bac 100644 --- a/orchestration/task_orchestrator.py +++ b/orchestration/task_orchestrator.py @@ -2,7 +2,7 @@ from typing import Any, Callable, Optional from uuid import UUID from domain.tasks.subtasks import SubTask -from domain.tasks.tasks import Source, Task, TaskStatus +from domain.tasks.tasks import Source, Task from repositories.tasks.subtask_repository import SubTaskRepository from repositories.tasks.task_repository import TaskRepository from utilities.private import private @@ -146,11 +146,10 @@ class TaskOrchestrator: @private def _cascade(self, task_id: UUID) -> None: task = self._tasks.get(task_id) - # FAILED is terminal: once any SubTask has failed the Task is failed and - # stays failed, so skip the (potentially large) sibling roll-up entirely — - # no need to list and re-check the SubTasks. - if task.status is TaskStatus.FAILED: - return + # Always recompute, even from FAILED: a failed batch SubTask may be + # re-run (re-sending its own inputs), and its completion must be able + # to un-fail the parent (ADR-0055). While any child is still failed, + # the recompute keeps the Task failed anyway. statuses = [s.status for s in self._subtasks.list_by_task(task_id)] task.recalculate_from_subtasks(statuses) self._tasks.save(task) From 561b8b078b9b71b9241e351dc682e87d29e0c20d Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:12:12 +0000 Subject: [PATCH 08/56] =?UTF-8?q?Test=20names=20state=20the=20surviving=20?= =?UTF-8?q?failure-recovery=20behaviors=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- tests/domain/tasks/test_subtasks.py | 2 +- tests/orchestration/test_task_orchestrator.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/domain/tasks/test_subtasks.py b/tests/domain/tasks/test_subtasks.py index d379439c6..18d09a6a5 100644 --- a/tests/domain/tasks/test_subtasks.py +++ b/tests/domain/tasks/test_subtasks.py @@ -49,7 +49,7 @@ def test_start_is_idempotent_from_in_progress() -> None: assert st.cloud_logs_url == "https://other" -def test_start_rejects_from_terminal_status() -> None: +def test_start_rejects_from_complete() -> None: # arrange st = SubTask.create(task_id=uuid4()) st.complete() diff --git a/tests/orchestration/test_task_orchestrator.py b/tests/orchestration/test_task_orchestrator.py index dc822e00c..fb0cd8e6c 100644 --- a/tests/orchestration/test_task_orchestrator.py +++ b/tests/orchestration/test_task_orchestrator.py @@ -286,9 +286,9 @@ def test_completing_a_rerun_failed_subtask_unfails_the_task( assert task_after.job_completed is not None -def test_cascade_short_circuits_once_task_already_failed(harness: Harness) -> None: - """Once the Task is FAILED, completing another SubTask leaves it FAILED — the - terminal state is not recomputed away.""" +def test_task_stays_failed_while_a_failed_subtask_remains(harness: Harness) -> None: + """Completing a *sibling* does not un-fail the Task — only re-running the + failed SubTask itself can (see the re-run test above).""" # arrange — two children; fail the first so the task is FAILED task, coordinator = harness.orchestrator.create_task_with_subtask( task_source="manual:test" From b008bcc6bac7c45a6152d83c12ebf1bbec36f2ff Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:32:09 +0000 Subject: [PATCH 09/56] The tasks mirror declares the FE-owned inputs column Co-Authored-By: Claude Fable 5 --- backend/app/db/models/tasks.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/backend/app/db/models/tasks.py b/backend/app/db/models/tasks.py index db1b7c04f..373a16b33 100644 --- a/backend/app/db/models/tasks.py +++ b/backend/app/db/models/tasks.py @@ -25,6 +25,11 @@ class Task(SQLModel, table=True): job_completed: Optional[datetime] = None status: str = Field(default="In Progress") service: Optional[str] = None + # FE-owned column (Drizzle migration): the app stores a task's original + # request here — for a Modelling Run, the full trigger-run payload + # (ADR-0055). The backend reads it at most; per-batch inputs live on the + # sub_tasks. + inputs: Optional[str] = None updated_at: datetime = Field(default_factory=datetime.utcnow) # source: Mapped[Optional[SourceEnum]] = mapped_column(Enum(SourceEnum)) <- SQLAlchemy not SQLModel From 84fb9884d6c02013eb60e40e4c320a0cdacc9d33 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:32:10 +0000 Subject: [PATCH 10/56] =?UTF-8?q?No=20filters=20resolves=20the=20whole=20p?= =?UTF-8?q?ortfolio=20minus=20deletions=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/__init__.py | 0 backend/app/modelling/property_filters.py | 44 +++++++++++++++++ infrastructure/postgres/property_table.py | 8 +++ tests/backend/__init__.py | 0 tests/backend/app/__init__.py | 0 tests/backend/app/modelling/__init__.py | 0 .../app/modelling/test_property_filters.py | 49 +++++++++++++++++++ 7 files changed, 101 insertions(+) create mode 100644 backend/app/modelling/__init__.py create mode 100644 backend/app/modelling/property_filters.py create mode 100644 tests/backend/__init__.py create mode 100644 tests/backend/app/__init__.py create mode 100644 tests/backend/app/modelling/__init__.py create mode 100644 tests/backend/app/modelling/test_property_filters.py diff --git a/backend/app/modelling/__init__.py b/backend/app/modelling/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py new file mode 100644 index 000000000..819ba3420 --- /dev/null +++ b/backend/app/modelling/property_filters.py @@ -0,0 +1,44 @@ +"""Resolve a Modelling Run's property-group filters to a concrete property set. + +The resolution rule is a shared contract with the app's preview (ADR-0056): +the "N properties will be modelled" count the user approves is computed by the +Next.js app from this same rule, so any change here must land in both +codebases and amend the ADR. +""" + +from dataclasses import dataclass +from typing import Optional + +from pydantic import BaseModel, ConfigDict +from sqlmodel import Session + + +class PropertyGroupFilters(BaseModel): + """The filters key of a trigger-run request. An absent key is + unconstrained; present keys combine with AND.""" + + model_config = ConfigDict(frozen=True) + + postcodes: Optional[list[str]] = None + property_types: Optional[list[str]] = None + built_forms: Optional[list[str]] = None + + +@dataclass(frozen=True) +class FilteredProperty: + """One property the filters resolved: the id to model and the postcode the + distributor batches by (postcode grouping keeps the workers' prediction + cohort cache effective).""" + + property_id: int + postcode: Optional[str] + + +def resolve_filtered_property_ids( + session: Session, portfolio_id: int, filters: PropertyGroupFilters +) -> list[FilteredProperty]: + """Resolve *filters* against *portfolio_id* per ADR-0056: base set is the + portfolio's rows with ``marked_for_deletion = false``; property_type / + built_form resolve override → lodged EPC → predicted EPC → legacy columns + → "Unknown".""" + raise NotImplementedError diff --git a/infrastructure/postgres/property_table.py b/infrastructure/postgres/property_table.py index 56d0b2fa7..c3cf497c2 100644 --- a/infrastructure/postgres/property_table.py +++ b/infrastructure/postgres/property_table.py @@ -56,3 +56,11 @@ class PropertyRow(SQLModel, table=True): # `updated_at >= 2026-06-01`, the cutoff the old pipeline predates). has_recommendations: Optional[bool] = Field(default=None) updated_at: Optional[datetime] = Field(default=None) + + # Columns the Modelling Run filter resolution reads (ADR-0056): the base-set + # exclusion flag and the legacy type/form columns (the last resort before + # "Unknown" in the resolution precedence). NOT NULL / defaulted in the FE + # schema; nullable mirrors are safe — prod enforces the real constraints. + marked_for_deletion: Optional[bool] = Field(default=None) + property_type: Optional[str] = Field(default=None) + built_form: Optional[str] = Field(default=None) diff --git a/tests/backend/__init__.py b/tests/backend/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/backend/app/__init__.py b/tests/backend/app/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/backend/app/modelling/__init__.py b/tests/backend/app/modelling/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py new file mode 100644 index 000000000..613200dba --- /dev/null +++ b/tests/backend/app/modelling/test_property_filters.py @@ -0,0 +1,49 @@ +from sqlalchemy import Engine +from sqlmodel import Session + +from backend.app.modelling.property_filters import ( + FilteredProperty, + PropertyGroupFilters, + resolve_filtered_property_ids, +) +from infrastructure.postgres.property_table import PropertyRow + + +def _seed_property( + session: Session, + *, + portfolio_id: int, + postcode: str = "B93 8SU", + marked_for_deletion: bool = False, +) -> int: + row = PropertyRow( + portfolio_id=portfolio_id, + postcode=postcode, + marked_for_deletion=marked_for_deletion, + ) + session.add(row) + session.commit() + session.refresh(row) + assert row.id is not None + return row.id + + +def test_no_filters_resolves_the_whole_portfolio_minus_deletions( + db_engine: Engine, +) -> None: + # arrange + with Session(db_engine) as session: + kept = _seed_property(session, portfolio_id=814, postcode="B93 8SU") + deleted = _seed_property( + session, portfolio_id=814, postcode="M20 4TF", marked_for_deletion=True + ) + _other_portfolio = _seed_property(session, portfolio_id=999) + + # act + resolved = resolve_filtered_property_ids( + session, portfolio_id=814, filters=PropertyGroupFilters() + ) + + # assert + assert resolved == [FilteredProperty(property_id=kept, postcode="B93 8SU")] + assert deleted not in [p.property_id for p in resolved] From 3836729f928018bb1625c11d2fbe514c3d88c7de Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:32:53 +0000 Subject: [PATCH 11/56] =?UTF-8?q?No=20filters=20resolves=20the=20whole=20p?= =?UTF-8?q?ortfolio=20minus=20deletions=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 819ba3420..8c5a26e8f 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -10,7 +10,9 @@ from dataclasses import dataclass from typing import Optional from pydantic import BaseModel, ConfigDict -from sqlmodel import Session +from sqlmodel import Session, col, select + +from infrastructure.postgres.property_table import PropertyRow class PropertyGroupFilters(BaseModel): @@ -41,4 +43,16 @@ def resolve_filtered_property_ids( portfolio's rows with ``marked_for_deletion = false``; property_type / built_form resolve override → lodged EPC → predicted EPC → legacy columns → "Unknown".""" - raise NotImplementedError + rows = session.exec( + select(PropertyRow) + .where(PropertyRow.portfolio_id == portfolio_id) + # IS NOT TRUE rather than == False: the FE schema defaults the flag, + # but a NULL must never silently exclude a property. + .where(col(PropertyRow.marked_for_deletion).isnot(True)) + .order_by(col(PropertyRow.id)) + ).all() + return [ + FilteredProperty(property_id=row.id, postcode=row.postcode) + for row in rows + if row.id is not None + ] From b865e89195d481f19eacea5072252bb1e96bc6e5 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:33:29 +0000 Subject: [PATCH 12/56] =?UTF-8?q?Postcode=20filter=20matches=20the=20canon?= =?UTF-8?q?ical=20postcode=20exactly=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 613200dba..6b3e62685 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -47,3 +47,20 @@ def test_no_filters_resolves_the_whole_portfolio_minus_deletions( # assert assert resolved == [FilteredProperty(property_id=kept, postcode="B93 8SU")] assert deleted not in [p.property_id for p in resolved] + + +def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: + # arrange + with Session(db_engine) as session: + in_area = _seed_property(session, portfolio_id=814, postcode="B93 8SU") + _elsewhere = _seed_property(session, portfolio_id=814, postcode="M20 4TF") + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(postcodes=["B93 8SU"]), + ) + + # assert + assert [p.property_id for p in resolved] == [in_area] From 10b9015bce09ae297785f79fba186c94be43fc24 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:34:07 +0000 Subject: [PATCH 13/56] =?UTF-8?q?Postcode=20filter=20matches=20the=20canon?= =?UTF-8?q?ical=20postcode=20exactly=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 8c5a26e8f..4f45c6325 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -43,14 +43,17 @@ def resolve_filtered_property_ids( portfolio's rows with ``marked_for_deletion = false``; property_type / built_form resolve override → lodged EPC → predicted EPC → legacy columns → "Unknown".""" - rows = session.exec( + statement = ( select(PropertyRow) .where(PropertyRow.portfolio_id == portfolio_id) # IS NOT TRUE rather than == False: the FE schema defaults the flag, # but a NULL must never silently exclude a property. .where(col(PropertyRow.marked_for_deletion).isnot(True)) .order_by(col(PropertyRow.id)) - ).all() + ) + if filters.postcodes is not None: + statement = statement.where(col(PropertyRow.postcode).in_(filters.postcodes)) + rows = session.exec(statement).all() return [ FilteredProperty(property_id=row.id, postcode=row.postcode) for row in rows From f40abcedeb796b94f4be3f54404e4c3b19115a17 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:36:13 +0000 Subject: [PATCH 14/56] =?UTF-8?q?Property-type=20override=20beats=20the=20?= =?UTF-8?q?lodged=20EPC=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 6b3e62685..3b2ec2f73 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -1,3 +1,5 @@ +from typing import Optional + from sqlalchemy import Engine from sqlmodel import Session @@ -6,6 +8,8 @@ from backend.app.modelling.property_filters import ( PropertyGroupFilters, resolve_filtered_property_ids, ) +from infrastructure.postgres.epc_property_table import EpcPropertyModel +from infrastructure.postgres.property_override_table import PropertyOverrideRow from infrastructure.postgres.property_table import PropertyRow @@ -28,6 +32,77 @@ def _seed_property( return row.id +def _seed_override( + session: Session, + *, + property_id: int, + component: str, + value: str, + building_part: int = 0, +) -> None: + session.add( + PropertyOverrideRow( + property_id=property_id, + portfolio_id=814, + building_part=building_part, + override_component=component, + override_value=value, + original_spreadsheet_description=value, + ) + ) + session.commit() + + +def _seed_epc( + session: Session, + *, + property_id: int, + source: str = "lodged", + property_type: Optional[str] = None, + built_form: Optional[str] = None, + dwelling_type: str = "Mid-terrace house", +) -> None: + session.add( + EpcPropertyModel( + property_id=property_id, + portfolio_id=814, + source=source, + dwelling_type=dwelling_type, + property_type=property_type, + built_form=built_form, + # Minimal NOT NULL ballast — irrelevant to filter resolution. + tenure="rental (social)", + transaction_type="assessment for green deal", + inspection_date="2024-01-01", + total_floor_area_m2=80.0, + solar_water_heating=False, + has_hot_water_cylinder=False, + has_fixed_air_conditioning=False, + door_count=1, + wet_rooms_count=1, + extensions_count=0, + heated_rooms_count=4, + open_chimneys_count=0, + habitable_rooms_count=4, + insulated_door_count=0, + cfl_fixed_lighting_bulbs_count=0, + led_fixed_lighting_bulbs_count=4, + incandescent_fixed_lighting_bulbs_count=0, + energy_gas_connection_available=True, + energy_meter_type="single", + energy_pv_battery_count=0, + energy_wind_turbines_count=0, + energy_gas_smart_meter_present=False, + energy_is_dwelling_export_capable=False, + energy_wind_turbines_terrain_type="", + energy_electricity_smart_meter_present=False, + energy_pv_diverter_present=False, + ventilation_present=False, + ) + ) + session.commit() + + def test_no_filters_resolves_the_whole_portfolio_minus_deletions( db_engine: Engine, ) -> None: @@ -49,6 +124,31 @@ def test_no_filters_resolves_the_whole_portfolio_minus_deletions( assert deleted not in [p.property_id for p in resolved] +def test_property_type_override_beats_the_lodged_epc(db_engine: Engine) -> None: + # arrange — both properties' lodged EPCs say House; one is overridden + with Session(db_engine) as session: + overridden = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=overridden, property_type="House") + _seed_override( + session, + property_id=overridden, + component="property_type", + value="Bungalow", + ) + plain = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=plain, property_type="House") + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Bungalow"]), + ) + + # assert + assert [p.property_id for p in resolved] == [overridden] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From 4967ddfb3ed062c480293d6fa7e2f20188cb3434 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:36:47 +0000 Subject: [PATCH 15/56] =?UTF-8?q?Property-type=20override=20beats=20the=20?= =?UTF-8?q?lodged=20EPC=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 49 ++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 4f45c6325..414f16889 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -12,6 +12,8 @@ from typing import Optional from pydantic import BaseModel, ConfigDict from sqlmodel import Session, col, select +from infrastructure.postgres.epc_property_table import EpcPropertyModel +from infrastructure.postgres.property_override_table import PropertyOverrideRow from infrastructure.postgres.property_table import PropertyRow @@ -54,8 +56,53 @@ def resolve_filtered_property_ids( if filters.postcodes is not None: statement = statement.where(col(PropertyRow.postcode).in_(filters.postcodes)) rows = session.exec(statement).all() + candidates = [row for row in rows if row.id is not None] + + if filters.property_types is not None: + wanted = set(filters.property_types) + types = _resolved_property_types( + session, [row.id for row in candidates if row.id is not None] + ) + candidates = [ + row + for row in candidates + if row.id is not None and types.get(row.id) in wanted + ] + return [ FilteredProperty(property_id=row.id, postcode=row.postcode) - for row in rows + for row in candidates if row.id is not None ] + + +def _resolved_property_types( + session: Session, property_ids: list[int] +) -> dict[int, str]: + """Each property's effective property_type per the ADR-0056 precedence: + override (building_part 0) → lodged EPC.""" + overrides = session.exec( + select(PropertyOverrideRow) + .where(col(PropertyOverrideRow.property_id).in_(property_ids)) + .where(PropertyOverrideRow.building_part == 0) + .where(PropertyOverrideRow.override_component == "property_type") + ).all() + by_override = {o.property_id: o.override_value for o in overrides} + + epcs = session.exec( + select(EpcPropertyModel) + .where(col(EpcPropertyModel.property_id).in_(property_ids)) + .where(EpcPropertyModel.source == "lodged") + ).all() + by_lodged_epc = { + epc.property_id: epc.property_type + for epc in epcs + if epc.property_id is not None and epc.property_type is not None + } + + resolved: dict[int, str] = {} + for pid in property_ids: + value = by_override.get(pid) or by_lodged_epc.get(pid) + if value is not None: + resolved[pid] = value + return resolved From 45dab78f4e751fd0e3678d3be58d807f91edc571 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:37:18 +0000 Subject: [PATCH 16/56] =?UTF-8?q?EPC-derived=20property=20type=20prefers?= =?UTF-8?q?=20lodged=20over=20predicted=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 3b2ec2f73..d7c77212a 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -149,6 +149,34 @@ def test_property_type_override_beats_the_lodged_epc(db_engine: Engine) -> None: assert [p.property_id for p in resolved] == [overridden] +def test_epc_property_type_prefers_lodged_over_predicted(db_engine: Engine) -> None: + # arrange — one property has both sources; the other only a prediction + with Session(db_engine) as session: + both = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=both, source="lodged", property_type="House") + _seed_epc( + session, property_id=both, source="predicted", property_type="Bungalow" + ) + predicted_only = _seed_property(session, portfolio_id=814) + _seed_epc( + session, + property_id=predicted_only, + source="predicted", + property_type="Bungalow", + ) + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Bungalow"]), + ) + + # assert — 'both' resolves House (lodged wins); only the prediction-backed + # property is a Bungalow + assert [p.property_id for p in resolved] == [predicted_only] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From e1dc0dc2f5f6bba65b0fe04b4362155f51a52b76 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:37:45 +0000 Subject: [PATCH 17/56] =?UTF-8?q?EPC-derived=20property=20type=20prefers?= =?UTF-8?q?=20lodged=20over=20predicted=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 414f16889..d7f715cdb 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -92,17 +92,20 @@ def _resolved_property_types( epcs = session.exec( select(EpcPropertyModel) .where(col(EpcPropertyModel.property_id).in_(property_ids)) - .where(EpcPropertyModel.source == "lodged") + .where(col(EpcPropertyModel.source).in_(("lodged", "predicted"))) ).all() - by_lodged_epc = { - epc.property_id: epc.property_type - for epc in epcs - if epc.property_id is not None and epc.property_type is not None - } + by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} + for epc in epcs: + if epc.property_id is not None and epc.property_type is not None: + by_epc_source[epc.source][epc.property_id] = epc.property_type resolved: dict[int, str] = {} for pid in property_ids: - value = by_override.get(pid) or by_lodged_epc.get(pid) + value = ( + by_override.get(pid) + or by_epc_source["lodged"].get(pid) + or by_epc_source["predicted"].get(pid) + ) if value is not None: resolved[pid] = value return resolved From 302861309c0aeb1de5a537c23f6d12c72b840ee2 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:38:18 +0000 Subject: [PATCH 18/56] =?UTF-8?q?Numeric=20RdSAP=20property-type=20codes?= =?UTF-8?q?=20map=20to=20their=20labels=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index d7c77212a..22d7a204b 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -177,6 +177,25 @@ def test_epc_property_type_prefers_lodged_over_predicted(db_engine: Engine) -> N assert [p.property_id for p in resolved] == [predicted_only] +def test_epc_numeric_property_type_codes_map_to_labels(db_engine: Engine) -> None: + # arrange — RdSAP code 1 = Bungalow + with Session(db_engine) as session: + coded = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=coded, property_type="1") + _house = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=_house, property_type="0") + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Bungalow"]), + ) + + # assert + assert [p.property_id for p in resolved] == [coded] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From dfe42a637414fa52c4a9e8a10bac977acd081a53 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:38:47 +0000 Subject: [PATCH 19/56] =?UTF-8?q?Numeric=20RdSAP=20property-type=20codes?= =?UTF-8?q?=20map=20to=20their=20labels=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index d7f715cdb..530a909cb 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -76,6 +76,17 @@ def resolve_filtered_property_ids( ] +# RdSAP numeric codes as lodged on some certs; text labels pass through as-is +# (ADR-0056 — the mapping is part of the shared preview contract). +_PROPERTY_TYPE_CODES: dict[str, str] = { + "0": "House", + "1": "Bungalow", + "2": "Flat", + "3": "Maisonette", + "4": "Park home", +} + + def _resolved_property_types( session: Session, property_ids: list[int] ) -> dict[int, str]: @@ -97,7 +108,10 @@ def _resolved_property_types( by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} for epc in epcs: if epc.property_id is not None and epc.property_type is not None: - by_epc_source[epc.source][epc.property_id] = epc.property_type + value = epc.property_type + by_epc_source[epc.source][epc.property_id] = _PROPERTY_TYPE_CODES.get( + value, value + ) resolved: dict[int, str] = {} for pid in property_ids: From 9b146dfaf43633e74af75aca3654077698bd1729 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:39:10 +0000 Subject: [PATCH 20/56] =?UTF-8?q?An=20EPC=20without=20property=5Ftype=20fa?= =?UTF-8?q?lls=20back=20to=20its=20dwelling=5Ftype=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 22d7a204b..b9439cf51 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -196,6 +196,30 @@ def test_epc_numeric_property_type_codes_map_to_labels(db_engine: Engine) -> Non assert [p.property_id for p in resolved] == [coded] +def test_epc_without_property_type_falls_back_to_dwelling_type( + db_engine: Engine, +) -> None: + # arrange — cert lodged with no property_type column, only dwelling_type + with Session(db_engine) as session: + fallback = _seed_property(session, portfolio_id=814) + _seed_epc( + session, + property_id=fallback, + property_type=None, + dwelling_type="Bungalow", + ) + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Bungalow"]), + ) + + # assert + assert [p.property_id for p in resolved] == [fallback] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From bf0e9343b2498e5bbef3da5b02753edb5888f03c Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:39:32 +0000 Subject: [PATCH 21/56] =?UTF-8?q?An=20EPC=20without=20property=5Ftype=20fa?= =?UTF-8?q?lls=20back=20to=20its=20dwelling=5Ftype=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 530a909cb..8a1428867 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -107,8 +107,8 @@ def _resolved_property_types( ).all() by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} for epc in epcs: - if epc.property_id is not None and epc.property_type is not None: - value = epc.property_type + value = epc.property_type or epc.dwelling_type + if epc.property_id is not None and value: by_epc_source[epc.source][epc.property_id] = _PROPERTY_TYPE_CODES.get( value, value ) From faea3209b267b74053b5cd6fee6c9a96587a8c28 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:39:54 +0000 Subject: [PATCH 22/56] =?UTF-8?q?A=20property=20without=20override=20or=20?= =?UTF-8?q?EPC=20falls=20back=20to=20the=20legacy=20column=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index b9439cf51..0ee70154e 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -220,6 +220,26 @@ def test_epc_without_property_type_falls_back_to_dwelling_type( assert [p.property_id for p in resolved] == [fallback] +def test_property_without_epc_falls_back_to_the_legacy_column( + db_engine: Engine, +) -> None: + # arrange — no override, no EPC rows; only property.property_type + with Session(db_engine) as session: + legacy = _seed_property(session, portfolio_id=814) + session.get_one(PropertyRow, legacy).property_type = "Bungalow" + session.commit() + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Bungalow"]), + ) + + # assert + assert [p.property_id for p in resolved] == [legacy] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From ec7ba1f0d3cc812fd5eceb05805743558ace6f32 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:40:24 +0000 Subject: [PATCH 23/56] =?UTF-8?q?A=20property=20without=20override=20or=20?= =?UTF-8?q?EPC=20falls=20back=20to=20the=20legacy=20column=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 8a1428867..b4ea91ddf 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -60,9 +60,7 @@ def resolve_filtered_property_ids( if filters.property_types is not None: wanted = set(filters.property_types) - types = _resolved_property_types( - session, [row.id for row in candidates if row.id is not None] - ) + types = _resolved_property_types(session, candidates) candidates = [ row for row in candidates @@ -88,10 +86,16 @@ _PROPERTY_TYPE_CODES: dict[str, str] = { def _resolved_property_types( - session: Session, property_ids: list[int] + session: Session, candidates: list[PropertyRow] ) -> dict[int, str]: """Each property's effective property_type per the ADR-0056 precedence: - override (building_part 0) → lodged EPC.""" + override (building_part 0) → lodged EPC → predicted EPC → legacy column.""" + property_ids = [row.id for row in candidates if row.id is not None] + by_legacy_column = { + row.id: row.property_type + for row in candidates + if row.id is not None and row.property_type is not None + } overrides = session.exec( select(PropertyOverrideRow) .where(col(PropertyOverrideRow.property_id).in_(property_ids)) @@ -119,6 +123,7 @@ def _resolved_property_types( by_override.get(pid) or by_epc_source["lodged"].get(pid) or by_epc_source["predicted"].get(pid) + or by_legacy_column.get(pid) ) if value is not None: resolved[pid] = value From 4d0af87d5a9a8c8cbdc2aeb07ddb00a67192628d Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:40:46 +0000 Subject: [PATCH 24/56] =?UTF-8?q?Unknown=20is=20a=20selectable=20property-?= =?UTF-8?q?type=20bucket=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 0ee70154e..223696dd5 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -240,6 +240,24 @@ def test_property_without_epc_falls_back_to_the_legacy_column( assert [p.property_id for p in resolved] == [legacy] +def test_unknown_is_a_selectable_property_type_bucket(db_engine: Engine) -> None: + # arrange — one property resolvable at no level, one resolvable + with Session(db_engine) as session: + unknowable = _seed_property(session, portfolio_id=814) + typed = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=typed, property_type="House") + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Unknown"]), + ) + + # assert + assert [p.property_id for p in resolved] == [unknowable] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From 92370504dac3bcd8d538243c08020a4fd3916c1d Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:41:19 +0000 Subject: [PATCH 25/56] =?UTF-8?q?Unknown=20is=20a=20selectable=20property-?= =?UTF-8?q?type=20bucket=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index b4ea91ddf..5f29e33ac 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -74,6 +74,10 @@ def resolve_filtered_property_ids( ] +# "Unknown" is itself a selectable filter value: exactly the bucket of +# properties whose value resolves at no precedence level (ADR-0056). +_UNKNOWN = "Unknown" + # RdSAP numeric codes as lodged on some certs; text labels pass through as-is # (ADR-0056 — the mapping is part of the shared preview contract). _PROPERTY_TYPE_CODES: dict[str, str] = { @@ -119,12 +123,11 @@ def _resolved_property_types( resolved: dict[int, str] = {} for pid in property_ids: - value = ( + resolved[pid] = ( by_override.get(pid) or by_epc_source["lodged"].get(pid) or by_epc_source["predicted"].get(pid) or by_legacy_column.get(pid) + or _UNKNOWN ) - if value is not None: - resolved[pid] = value return resolved From d56079a1d45d357fd136fa56f6378313dbe47096 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:41:47 +0000 Subject: [PATCH 26/56] =?UTF-8?q?Built-form=20filter=20resolves=20with=20t?= =?UTF-8?q?he=20same=20precedence=20as=20property=20type=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 223696dd5..7f4a0f255 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -258,6 +258,35 @@ def test_unknown_is_a_selectable_property_type_bucket(db_engine: Engine) -> None assert [p.property_id for p in resolved] == [unknowable] +def test_built_form_filter_resolves_with_the_same_precedence( + db_engine: Engine, +) -> None: + # arrange — both lodged as code 4 (Mid-Terrace); one overridden to Detached + with Session(db_engine) as session: + overridden = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=overridden, built_form="4") + _seed_override( + session, + property_id=overridden, + component="built_form_type", + value="Detached", + ) + mid_terrace = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=mid_terrace, built_form="4") + semi = _seed_property(session, portfolio_id=814) + _seed_epc(session, property_id=semi, built_form="2") + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(built_forms=["Detached", "Mid-Terrace"]), + ) + + # assert — the override wins for one; the code maps for the other + assert [p.property_id for p in resolved] == [overridden, mid_terrace] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From fc3f6a6f9472366a6a78fffbf3e0985f063b7d83 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:43:03 +0000 Subject: [PATCH 27/56] =?UTF-8?q?Built-form=20filter=20resolves=20with=20t?= =?UTF-8?q?he=20same=20precedence=20as=20property=20type=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 62 +++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 5f29e33ac..a09aedb94 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -67,6 +67,15 @@ def resolve_filtered_property_ids( if row.id is not None and types.get(row.id) in wanted ] + if filters.built_forms is not None: + wanted_forms = set(filters.built_forms) + forms = _resolved_built_forms(session, candidates) + candidates = [ + row + for row in candidates + if row.id is not None and forms.get(row.id) in wanted_forms + ] + return [ FilteredProperty(property_id=row.id, postcode=row.postcode) for row in candidates @@ -88,6 +97,59 @@ _PROPERTY_TYPE_CODES: dict[str, str] = { "4": "Park home", } +_BUILT_FORM_CODES: dict[str, str] = { + "1": "Detached", + "2": "Semi-Detached", + "3": "End-Terrace", + "4": "Mid-Terrace", + "5": "Enclosed End-Terrace", + "6": "Enclosed Mid-Terrace", +} + + +def _resolved_built_forms( + session: Session, candidates: list[PropertyRow] +) -> dict[int, str]: + """Each property's effective built_form per the ADR-0056 precedence: + override (building_part 0) → lodged EPC → predicted EPC → legacy column.""" + property_ids = [row.id for row in candidates if row.id is not None] + by_legacy_column = { + row.id: row.built_form + for row in candidates + if row.id is not None and row.built_form is not None + } + overrides = session.exec( + select(PropertyOverrideRow) + .where(col(PropertyOverrideRow.property_id).in_(property_ids)) + .where(PropertyOverrideRow.building_part == 0) + .where(PropertyOverrideRow.override_component == "built_form_type") + ).all() + by_override = {o.property_id: o.override_value for o in overrides} + + epcs = session.exec( + select(EpcPropertyModel) + .where(col(EpcPropertyModel.property_id).in_(property_ids)) + .where(col(EpcPropertyModel.source).in_(("lodged", "predicted"))) + ).all() + by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} + for epc in epcs: + if epc.property_id is not None and epc.built_form: + value = epc.built_form + by_epc_source[epc.source][epc.property_id] = _BUILT_FORM_CODES.get( + value, value + ) + + resolved: dict[int, str] = {} + for pid in property_ids: + resolved[pid] = ( + by_override.get(pid) + or by_epc_source["lodged"].get(pid) + or by_epc_source["predicted"].get(pid) + or by_legacy_column.get(pid) + or _UNKNOWN + ) + return resolved + def _resolved_property_types( session: Session, candidates: list[PropertyRow] From 513977a6fd6020c208db0048fca048396b20ed8b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:43:51 +0000 Subject: [PATCH 28/56] =?UTF-8?q?Filters=20combine=20with=20AND=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior emerged from filter composition — pinned straight to green. Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index 7f4a0f255..b7d029368 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -287,6 +287,30 @@ def test_built_form_filter_resolves_with_the_same_precedence( assert [p.property_id for p in resolved] == [overridden, mid_terrace] +def test_filters_combine_with_and(db_engine: Engine) -> None: + # arrange — right postcode + right type, right postcode + wrong type, + # wrong postcode + right type + with Session(db_engine) as session: + match = _seed_property(session, portfolio_id=814, postcode="B93 8SU") + _seed_epc(session, property_id=match, property_type="House") + wrong_type = _seed_property(session, portfolio_id=814, postcode="B93 8SU") + _seed_epc(session, property_id=wrong_type, property_type="Flat") + wrong_postcode = _seed_property(session, portfolio_id=814, postcode="M20 4TF") + _seed_epc(session, property_id=wrong_postcode, property_type="House") + + # act + resolved = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters( + postcodes=["B93 8SU"], property_types=["House"] + ), + ) + + # assert + assert [p.property_id for p in resolved] == [match] + + def test_postcode_filter_matches_exactly(db_engine: Engine) -> None: # arrange with Session(db_engine) as session: From 19d9e758adbdbf2f669c670b0c0f41687432d3d7 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:45:10 +0000 Subject: [PATCH 29/56] =?UTF-8?q?Both=20filterable=20components=20resolve?= =?UTF-8?q?=20through=20one=20precedence=20engine=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 178 ++++++++++------------ 1 file changed, 78 insertions(+), 100 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index a09aedb94..ce398cb25 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -6,6 +6,7 @@ Next.js app from this same rule, so any change here must land in both codebases and amend the ADR. """ +from collections.abc import Callable from dataclasses import dataclass from typing import Optional @@ -38,13 +39,60 @@ class FilteredProperty: postcode: Optional[str] +# "Unknown" is itself a selectable filter value: exactly the bucket of +# properties whose value resolves at no precedence level (ADR-0056). +_UNKNOWN = "Unknown" + + +@dataclass(frozen=True) +class _ComponentResolution: + """How one filterable component (property_type / built_form) resolves at + each ADR-0056 precedence level: which override row names it, how its RdSAP + numeric codes map (text labels pass through as-is), and where it lives on + the EPC and the legacy property row.""" + + override_component: str + codes: dict[str, str] + epc_value: Callable[[EpcPropertyModel], Optional[str]] + legacy_value: Callable[[PropertyRow], Optional[str]] + + +_PROPERTY_TYPE = _ComponentResolution( + override_component="property_type", + codes={ + "0": "House", + "1": "Bungalow", + "2": "Flat", + "3": "Maisonette", + "4": "Park home", + }, + # A cert without the property_type column falls back to its dwelling_type. + epc_value=lambda epc: epc.property_type or epc.dwelling_type, + legacy_value=lambda row: row.property_type, +) + +_BUILT_FORM = _ComponentResolution( + override_component="built_form_type", + codes={ + "1": "Detached", + "2": "Semi-Detached", + "3": "End-Terrace", + "4": "Mid-Terrace", + "5": "Enclosed End-Terrace", + "6": "Enclosed Mid-Terrace", + }, + epc_value=lambda epc: epc.built_form, + legacy_value=lambda row: row.built_form, +) + + def resolve_filtered_property_ids( session: Session, portfolio_id: int, filters: PropertyGroupFilters ) -> list[FilteredProperty]: """Resolve *filters* against *portfolio_id* per ADR-0056: base set is the portfolio's rows with ``marked_for_deletion = false``; property_type / built_form resolve override → lodged EPC → predicted EPC → legacy columns - → "Unknown".""" + → "Unknown". Filters AND-combine; an absent key is unconstrained.""" statement = ( select(PropertyRow) .where(PropertyRow.portfolio_id == portfolio_id) @@ -55,25 +103,20 @@ def resolve_filtered_property_ids( ) if filters.postcodes is not None: statement = statement.where(col(PropertyRow.postcode).in_(filters.postcodes)) - rows = session.exec(statement).all() - candidates = [row for row in rows if row.id is not None] + candidates = [row for row in session.exec(statement).all() if row.id is not None] - if filters.property_types is not None: - wanted = set(filters.property_types) - types = _resolved_property_types(session, candidates) + for wanted_values, component in ( + (filters.property_types, _PROPERTY_TYPE), + (filters.built_forms, _BUILT_FORM), + ): + if wanted_values is None: + continue + wanted = set(wanted_values) + resolved = _resolved_component_values(session, candidates, component) candidates = [ row for row in candidates - if row.id is not None and types.get(row.id) in wanted - ] - - if filters.built_forms is not None: - wanted_forms = set(filters.built_forms) - forms = _resolved_built_forms(session, candidates) - candidates = [ - row - for row in candidates - if row.id is not None and forms.get(row.id) in wanted_forms + if row.id is not None and resolved.get(row.id) in wanted ] return [ @@ -83,46 +126,19 @@ def resolve_filtered_property_ids( ] -# "Unknown" is itself a selectable filter value: exactly the bucket of -# properties whose value resolves at no precedence level (ADR-0056). -_UNKNOWN = "Unknown" - -# RdSAP numeric codes as lodged on some certs; text labels pass through as-is -# (ADR-0056 — the mapping is part of the shared preview contract). -_PROPERTY_TYPE_CODES: dict[str, str] = { - "0": "House", - "1": "Bungalow", - "2": "Flat", - "3": "Maisonette", - "4": "Park home", -} - -_BUILT_FORM_CODES: dict[str, str] = { - "1": "Detached", - "2": "Semi-Detached", - "3": "End-Terrace", - "4": "Mid-Terrace", - "5": "Enclosed End-Terrace", - "6": "Enclosed Mid-Terrace", -} - - -def _resolved_built_forms( - session: Session, candidates: list[PropertyRow] +def _resolved_component_values( + session: Session, candidates: list[PropertyRow], component: _ComponentResolution ) -> dict[int, str]: - """Each property's effective built_form per the ADR-0056 precedence: - override (building_part 0) → lodged EPC → predicted EPC → legacy column.""" + """Each candidate's effective value for *component* per the ADR-0056 + precedence: override (building_part 0) → lodged EPC → predicted EPC → + legacy column → "Unknown".""" property_ids = [row.id for row in candidates if row.id is not None] - by_legacy_column = { - row.id: row.built_form - for row in candidates - if row.id is not None and row.built_form is not None - } + overrides = session.exec( select(PropertyOverrideRow) .where(col(PropertyOverrideRow.property_id).in_(property_ids)) .where(PropertyOverrideRow.building_part == 0) - .where(PropertyOverrideRow.override_component == "built_form_type") + .where(PropertyOverrideRow.override_component == component.override_component) ).all() by_override = {o.property_id: o.override_value for o in overrides} @@ -133,63 +149,25 @@ def _resolved_built_forms( ).all() by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} for epc in epcs: - if epc.property_id is not None and epc.built_form: - value = epc.built_form - by_epc_source[epc.source][epc.property_id] = _BUILT_FORM_CODES.get( - value, value - ) - - resolved: dict[int, str] = {} - for pid in property_ids: - resolved[pid] = ( - by_override.get(pid) - or by_epc_source["lodged"].get(pid) - or by_epc_source["predicted"].get(pid) - or by_legacy_column.get(pid) - or _UNKNOWN - ) - return resolved - - -def _resolved_property_types( - session: Session, candidates: list[PropertyRow] -) -> dict[int, str]: - """Each property's effective property_type per the ADR-0056 precedence: - override (building_part 0) → lodged EPC → predicted EPC → legacy column.""" - property_ids = [row.id for row in candidates if row.id is not None] - by_legacy_column = { - row.id: row.property_type - for row in candidates - if row.id is not None and row.property_type is not None - } - overrides = session.exec( - select(PropertyOverrideRow) - .where(col(PropertyOverrideRow.property_id).in_(property_ids)) - .where(PropertyOverrideRow.building_part == 0) - .where(PropertyOverrideRow.override_component == "property_type") - ).all() - by_override = {o.property_id: o.override_value for o in overrides} - - epcs = session.exec( - select(EpcPropertyModel) - .where(col(EpcPropertyModel.property_id).in_(property_ids)) - .where(col(EpcPropertyModel.source).in_(("lodged", "predicted"))) - ).all() - by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} - for epc in epcs: - value = epc.property_type or epc.dwelling_type + value = component.epc_value(epc) if epc.property_id is not None and value: - by_epc_source[epc.source][epc.property_id] = _PROPERTY_TYPE_CODES.get( + by_epc_source[epc.source][epc.property_id] = component.codes.get( value, value ) - resolved: dict[int, str] = {} - for pid in property_ids: - resolved[pid] = ( + by_legacy_column = { + row.id: value + for row in candidates + if row.id is not None and (value := component.legacy_value(row)) is not None + } + + return { + pid: ( by_override.get(pid) or by_epc_source["lodged"].get(pid) or by_epc_source["predicted"].get(pid) or by_legacy_column.get(pid) or _UNKNOWN ) - return resolved + for pid in property_ids + } From ca6ca2a220b32d7fbc71f5aa479db277882f697b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:47:20 +0000 Subject: [PATCH 30/56] =?UTF-8?q?A=20batch=20message=20with=20task=5Fid=20?= =?UTF-8?q?and=20subtask=5Fid=20attaches=20to=20the=20app-owned=20task=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../utilities/aws_lambda/test_task_handler.py | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/utilities/aws_lambda/test_task_handler.py b/tests/utilities/aws_lambda/test_task_handler.py index 418669bcd..974a47966 100644 --- a/tests/utilities/aws_lambda/test_task_handler.py +++ b/tests/utilities/aws_lambda/test_task_handler.py @@ -104,6 +104,60 @@ def test_task_handler_passes_orchestrator_and_task_id_when_flag_is_true( assert recv_task_id == UUID(result[0]["task_id"]) +def test_task_handler_attaches_to_a_supplied_task_and_subtask( + harness: Harness, db_engine: Engine +) -> None: + """A message carrying task_id + subtask_id (a Modelling Run batch, + ADR-0055) runs under the distributor's pre-created sub_task — no new Task + or SubTask rows are created.""" + # arrange — the distributor's pre-created task + batch sub_task + task, subtask = harness.orchestrator.create_task_with_subtask( + task_source="app:modelling_run", inputs={"property_ids": [1, 2]} + ) + received: list[UUID] = [] + + @task_handler( + task_source="modelling_e2e", + source=Source.PROPERTY, + orchestrator_cm=harness.factory, + pass_task_orchestrator=True, + ) + def handler( + body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator, task_id: UUID + ) -> None: + received.append(task_id) + + event = { + "Records": [ + { + "messageId": "m-1", + "body": ( + f'{{"task_id": "{task.id}", "subtask_id": "{subtask.id}", ' + f'"property_ids": [1, 2]}}' + ), + } + ] + } + + # act + result = handler(event, context=None) + + # assert — ran under the supplied ids, sub_task completed, nothing created + assert received == [task.id] + assert result["tasks"] == [ + {"task_id": str(task.id), "subtask_id": str(subtask.id)} + ] + assert harness.subtasks.get(subtask.id).status.value == "complete" + with Session(db_engine) as session: + from sqlmodel import select + + from backend.app.db.models.tasks import SubTask as SubTaskRow + from backend.app.db.models.tasks import Task as TaskRow + + assert len(session.exec(select(TaskRow)).all()) == 1 + assert len(session.exec(select(SubTaskRow)).all()) == 1 + + def test_task_handler_leaves_cloudwatch_url_unset_outside_lambda( harness: Harness, monkeypatch: pytest.MonkeyPatch ) -> None: From f01131a064f16b61dd7e15438df45bb2bf93fb78 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:48:53 +0000 Subject: [PATCH 31/56] =?UTF-8?q?A=20batch=20message=20with=20task=5Fid=20?= =?UTF-8?q?and=20subtask=5Fid=20attaches=20to=20the=20app-owned=20task=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../utilities/aws_lambda/test_task_handler.py | 13 ++--- utilities/aws_lambda/task_handler.py | 51 +++++++++++++------ 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/tests/utilities/aws_lambda/test_task_handler.py b/tests/utilities/aws_lambda/test_task_handler.py index 974a47966..b986c1c5e 100644 --- a/tests/utilities/aws_lambda/test_task_handler.py +++ b/tests/utilities/aws_lambda/test_task_handler.py @@ -5,7 +5,7 @@ from typing import Any from uuid import UUID import pytest -from sqlalchemy import Engine +from sqlalchemy import Engine, text from sqlmodel import Session from domain.tasks.tasks import Source @@ -148,14 +148,9 @@ def test_task_handler_attaches_to_a_supplied_task_and_subtask( {"task_id": str(task.id), "subtask_id": str(subtask.id)} ] assert harness.subtasks.get(subtask.id).status.value == "complete" - with Session(db_engine) as session: - from sqlmodel import select - - from backend.app.db.models.tasks import SubTask as SubTaskRow - from backend.app.db.models.tasks import Task as TaskRow - - assert len(session.exec(select(TaskRow)).all()) == 1 - assert len(session.exec(select(SubTaskRow)).all()) == 1 + with db_engine.connect() as conn: + assert conn.execute(text("SELECT count(*) FROM tasks")).scalar_one() == 1 + assert conn.execute(text("SELECT count(*) FROM sub_task")).scalar_one() == 1 def test_task_handler_leaves_cloudwatch_url_unset_outside_lambda( diff --git a/utilities/aws_lambda/task_handler.py b/utilities/aws_lambda/task_handler.py index f2064f573..695153add 100644 --- a/utilities/aws_lambda/task_handler.py +++ b/utilities/aws_lambda/task_handler.py @@ -9,6 +9,7 @@ import logging from contextlib import AbstractContextManager from functools import wraps from typing import Any, Callable, Optional, cast +from uuid import UUID from utilities.aws_lambda.cloud_logs import cloudwatch_url from utilities.aws_lambda.default_orchestrator import default_orchestrator @@ -50,39 +51,47 @@ def task_handler( for record in _records(event): body = _parse_body(record) - raw_source_id = body.get(source.value) - source_id = ( - str(raw_source_id) if raw_source_id is not None else None - ) - task, subtask = orchestrator.create_task_with_subtask( - task_source=task_source, - inputs=body, - source=source, - source_id=source_id, - ) + # Attach mode (ADR-0055): a Modelling Run batch carries the + # app-owned task_id and its distributor-pre-created + # subtask_id — run under those; create nothing. + supplied = _supplied_ids(body) + if supplied is not None: + task_id, subtask_id = supplied + else: + raw_source_id = body.get(source.value) + source_id = ( + str(raw_source_id) if raw_source_id is not None else None + ) + task, subtask = orchestrator.create_task_with_subtask( + task_source=task_source, + inputs=body, + source=source, + source_id=source_id, + ) + task_id, subtask_id = task.id, subtask.id task_ids.append( - {"task_id": str(task.id), "subtask_id": str(subtask.id)} + {"task_id": str(task_id), "subtask_id": str(subtask_id)} ) try: if pass_task_orchestrator: orchestrator.run_subtask( - subtask.id, - work=lambda: func(body, context, orchestrator, task.id), + subtask_id, + work=lambda: func(body, context, orchestrator, task_id), cloud_logs_url=cloud_logs_url, ) else: orchestrator.run_subtask( - subtask.id, + subtask_id, work=lambda: func(body, context), cloud_logs_url=cloud_logs_url, ) except Exception: logger.exception( - "subtask failed (task_source=%s source_id=%s)", + "subtask failed (task_source=%s subtask_id=%s)", task_source, - source_id, + subtask_id, ) if "Records" in event: message_id = record.get("messageId", "") @@ -99,6 +108,16 @@ def task_handler( return decorator +def _supplied_ids(body: dict[str, Any]) -> Optional[tuple[UUID, UUID]]: + """The (task_id, subtask_id) an attach-mode message carries, or None for + the classic create-your-own-task message.""" + raw_task_id = body.get("task_id") + raw_subtask_id = body.get("subtask_id") + if raw_task_id is None or raw_subtask_id is None: + return None + return UUID(str(raw_task_id)), UUID(str(raw_subtask_id)) + + def _parse_body(record: dict[str, Any]) -> dict[str, Any]: raw = record.get("body", record) if isinstance(raw, str): From 8a3c0f3e77f786efcfd431582418a8c8c0e07fbe Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:49:56 +0000 Subject: [PATCH 32/56] =?UTF-8?q?A=20recorded=20batch=20failure=20carries?= =?UTF-8?q?=20structured=20details=20onto=20the=20sub=5Ftask=20outputs=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- domain/tasks/subtasks.py | 13 +++++++++++++ tests/domain/tasks/test_subtasks.py | 27 ++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/domain/tasks/subtasks.py b/domain/tasks/subtasks.py index 95f9ec5bc..edb3ee508 100644 --- a/domain/tasks/subtasks.py +++ b/domain/tasks/subtasks.py @@ -5,6 +5,19 @@ from typing import Any, Optional from uuid import UUID, uuid4 +class SubTaskFailure(Exception): + """A *recorded* failure: the work ran, its outcome belongs on the SubTask + record, and the message must NOT be retried (ADR-0055 — status is a + record, not retry fuel). Infra crashes raise anything else and keep their + retry semantics. ``details`` lands on the failed SubTask's outputs.""" + + def __init__( + self, message: str, details: Optional[dict[str, Any]] = None + ) -> None: + super().__init__(message) + self.details = details + + class SubTaskStatus(str, Enum): WAITING = "waiting" IN_PROGRESS = "in progress" diff --git a/tests/domain/tasks/test_subtasks.py b/tests/domain/tasks/test_subtasks.py index 18d09a6a5..8d000ba96 100644 --- a/tests/domain/tasks/test_subtasks.py +++ b/tests/domain/tasks/test_subtasks.py @@ -2,7 +2,7 @@ from uuid import uuid4 import pytest -from domain.tasks.subtasks import SubTask, SubTaskStatus +from domain.tasks.subtasks import SubTask, SubTaskFailure, SubTaskStatus def test_create_subtask_starts_waiting() -> None: @@ -95,6 +95,31 @@ def test_complete_without_result_leaves_outputs_unset() -> None: assert st.outputs is None +def test_fail_with_subtask_failure_records_structured_details() -> None: + # arrange + st = SubTask.create(task_id=uuid4()) + st.start() + + # act + st.fail( + SubTaskFailure( + "1 of 2 properties failed", + details={ + "succeeded": 1, + "failed": [{"property_id": 9, "error": "degenerate prediction"}], + }, + ) + ) + + # assert + assert st.status is SubTaskStatus.FAILED + assert st.outputs == { + "error": "1 of 2 properties failed", + "succeeded": 1, + "failed": [{"property_id": 9, "error": "degenerate prediction"}], + } + + def test_fail_records_error_in_outputs() -> None: # arrange st = SubTask.create(task_id=uuid4()) From c64bd8fd85110762d6b3c07449ae4087dcdaef85 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:50:10 +0000 Subject: [PATCH 33/56] =?UTF-8?q?A=20recorded=20batch=20failure=20carries?= =?UTF-8?q?=20structured=20details=20onto=20the=20sub=5Ftask=20outputs=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- domain/tasks/subtasks.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/domain/tasks/subtasks.py b/domain/tasks/subtasks.py index edb3ee508..a0e2f985c 100644 --- a/domain/tasks/subtasks.py +++ b/domain/tasks/subtasks.py @@ -69,3 +69,5 @@ class SubTask: self.status = SubTaskStatus.FAILED self.job_completed = datetime.now(timezone.utc) self.outputs = {"error": str(error)} + if isinstance(error, SubTaskFailure) and error.details is not None: + self.outputs.update(error.details) From 52b07e3ea498fcfd350c8cd5dd8dfb9bee325204 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:50:38 +0000 Subject: [PATCH 34/56] =?UTF-8?q?A=20recorded=20batch=20failure=20does=20n?= =?UTF-8?q?ot=20return=20the=20message=20to=20SQS=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../utilities/aws_lambda/test_task_handler.py | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/utilities/aws_lambda/test_task_handler.py b/tests/utilities/aws_lambda/test_task_handler.py index b986c1c5e..2bacf38e0 100644 --- a/tests/utilities/aws_lambda/test_task_handler.py +++ b/tests/utilities/aws_lambda/test_task_handler.py @@ -8,6 +8,7 @@ import pytest from sqlalchemy import Engine, text from sqlmodel import Session +from domain.tasks.subtasks import SubTaskFailure from domain.tasks.tasks import Source from orchestration.task_orchestrator import TaskOrchestrator from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository @@ -153,6 +154,50 @@ def test_task_handler_attaches_to_a_supplied_task_and_subtask( assert conn.execute(text("SELECT count(*) FROM sub_task")).scalar_one() == 1 +def test_recorded_failure_fails_the_subtask_without_an_sqs_retry( + harness: Harness, +) -> None: + """A SubTaskFailure is a database record, not retry fuel (ADR-0055): the + sub_task fails with the structured details, but the message is NOT + reported to SQS as a batch item failure.""" + # arrange + task, subtask = harness.orchestrator.create_task_with_subtask( + task_source="app:modelling_run", inputs={"property_ids": [1, 2]} + ) + + @task_handler( + task_source="modelling_e2e", + source=Source.PROPERTY, + orchestrator_cm=harness.factory, + ) + def handler(body: dict[str, Any], context: Any) -> None: + raise SubTaskFailure( + "1 of 2 properties failed", + details={"succeeded": 1, "failed": [{"property_id": 2, "error": "x"}]}, + ) + + event = { + "Records": [ + { + "messageId": "m-1", + "body": ( + f'{{"task_id": "{task.id}", "subtask_id": "{subtask.id}", ' + f'"property_ids": [1, 2]}}' + ), + } + ] + } + + # act + result = handler(event, context=None) + + # assert — recorded, not retried + assert result["batchItemFailures"] == [] + failed = harness.subtasks.get(subtask.id) + assert failed.status.value == "failed" + assert failed.outputs is not None and failed.outputs["succeeded"] == 1 + + def test_task_handler_leaves_cloudwatch_url_unset_outside_lambda( harness: Harness, monkeypatch: pytest.MonkeyPatch ) -> None: From f317233939ffb797c70fae06d0345f99ec0b8880 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:51:23 +0000 Subject: [PATCH 35/56] =?UTF-8?q?A=20recorded=20batch=20failure=20does=20n?= =?UTF-8?q?ot=20return=20the=20message=20to=20SQS=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- utilities/aws_lambda/task_handler.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/utilities/aws_lambda/task_handler.py b/utilities/aws_lambda/task_handler.py index 695153add..de191ffb9 100644 --- a/utilities/aws_lambda/task_handler.py +++ b/utilities/aws_lambda/task_handler.py @@ -13,6 +13,7 @@ from uuid import UUID from utilities.aws_lambda.cloud_logs import cloudwatch_url from utilities.aws_lambda.default_orchestrator import default_orchestrator +from domain.tasks.subtasks import SubTaskFailure from domain.tasks.tasks import Source from orchestration.task_orchestrator import TaskOrchestrator @@ -87,6 +88,19 @@ def task_handler( work=lambda: func(body, context), cloud_logs_url=cloud_logs_url, ) + except SubTaskFailure as recorded: + # A recorded failure (ADR-0055): run_subtask has already + # failed the SubTask with the structured details — that + # record IS the outcome. Never returned to SQS for retry; + # recovery is a deliberate re-send of the sub_task's + # own inputs. + logger.warning( + "subtask recorded failure, not retrying " + "(task_source=%s subtask_id=%s): %s", + task_source, + subtask_id, + recorded, + ) except Exception: logger.exception( "subtask failed (task_source=%s subtask_id=%s)", From b840af11de0086e4bbeb3a373fc4c0a931242d6a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:52:45 +0000 Subject: [PATCH 36/56] =?UTF-8?q?An=20attach-mode=20batch=20models=20under?= =?UTF-8?q?=20the=20supplied=20sub=5Ftask=20without=20children=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../modelling_e2e/test_handler.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index 80c44aeeb..fe7ef7815 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -378,6 +378,66 @@ def test_handler_creates_one_child_subtask_per_property_id() -> None: assert [i["property_id"] for i in inputs_per_subtask] == [pid1, pid2, pid3] +def test_attach_mode_models_the_batch_without_child_subtasks() -> None: + """A batch carrying task_id + subtask_id (a Modelling Run, ADR-0055) runs + entirely under the distributor's pre-created sub_task: no per-property + child SubTasks are created, and the batch still persists.""" + # Arrange + pid1, pid2 = 111, 222 + mock_engine = _engine_mock([pid1, pid2], [1001, 1002], [POSTCODE, POSTCODE]) + mock_orch = _mock_orchestrator() + task_id = uuid4() + + with ExitStack() as stack: + stack.enter_context(patch("applications.modelling_e2e.handler.os.environ", _ENV)) + stack.enter_context( + patch("applications.modelling_e2e.handler._get_engine", return_value=mock_engine) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler.EpcClientService") + ).return_value.get_by_uprn.return_value = MagicMock() + stack.enter_context(patch("applications.modelling_e2e.handler.GeospatialS3Repository")) + stack.enter_context(patch("applications.modelling_e2e.handler.GoogleSolarApiClient")) + stack.enter_context( + patch("applications.modelling_e2e.handler._spatial_for", return_value=None) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler._solar_insights_for", return_value=None) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler.overlays_from", return_value=[]) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler.PropertyOverridesPostgresReader") + ).return_value.overrides_for_many.return_value = {} + stack.enter_context( + patch("applications.modelling_e2e.handler.ScenarioPostgresRepository") + ).return_value.get_many.return_value = [MagicMock()] + stack.enter_context(patch("applications.modelling_e2e.handler.catalogue_snapshot_with_off_catalogue_overrides")) + stack.enter_context(patch("applications.modelling_e2e.handler.Session")) + stack.enter_context( + patch("applications.modelling_e2e.handler.run_modelling", return_value=_plan_mock()) + ) + MockUoW = stack.enter_context(patch("applications.modelling_e2e.handler.PostgresUnitOfWork")) + mock_uow = MagicMock() + MockUoW.return_value.__enter__.return_value = mock_uow + MockUoW.return_value.__exit__.return_value = False + + # Act + from applications.modelling_e2e.handler import handler + handler.__wrapped__( # type: ignore[attr-defined] + {"task_id": str(task_id), "subtask_id": str(uuid4()), + "property_ids": [pid1, pid2], "portfolio_id": PORTFOLIO_ID, + "scenario_id": SCENARIO_ID, "refetch_solar": False, "dry_run": False}, + None, mock_orch, task_id, + ) + + # Assert — no child SubTasks; the whole batch persisted in the one UoW + mock_orch.run_subtasks.assert_not_called() + plan_requests = mock_uow.plan.save_batch.call_args.args[0] + assert [r.property_id for r in plan_requests] == [pid1, pid2] + + # --------------------------------------------------------------------------- # Lodged EPC path # --------------------------------------------------------------------------- From 770951843147332371da6985030521ba8a4a3c65 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:53:26 +0000 Subject: [PATCH 37/56] =?UTF-8?q?An=20attach-mode=20batch=20models=20under?= =?UTF-8?q?=20the=20supplied=20sub=5Ftask=20without=20children=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- applications/modelling_e2e/handler.py | 31 ++++++++++++------- .../modelling_e2e_trigger_body.py | 7 +++++ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index bf9ca8b06..9215a078c 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -536,7 +536,9 @@ def handler( def _work(subtask: SubTask) -> None: inputs = subtask.inputs or {} - pid = int(inputs["property_id"]) + _model_property(int(inputs["property_id"])) + + def _model_property(pid: int) -> None: uprn = uprns[pid] postcode = postcodes.get(pid, "") logger.info(f"property={pid} uprn={uprn} postcode={postcode!r}") @@ -685,16 +687,23 @@ def handler( ) logger.info(f"property={pid} queued for write") - # Fan the batch out into one child SubTask per property and run them in - # a single batched pass: create all children, model each (failures - # isolated per child), then persist all their statuses in two writes + - # one cascade — not ~5 writes and a full parent re-roll-up per property - # (see TaskOrchestrator.run_subtasks). - orchestrator.run_subtasks( - task_id, - [{"property_id": pid} for pid in property_ids], - work=_work, - ) + if trigger.subtask_id is not None: + # Attach mode (ADR-0055): the whole batch runs under the + # distributor's pre-created sub_task — the @task_handler wrapper is + # already inside it — so no per-property children are created. + for pid in property_ids: + _model_property(pid) + else: + # Fan the batch out into one child SubTask per property and run + # them in a single batched pass: create all children, model each + # (failures isolated per child), then persist all their statuses in + # two writes + one cascade — not ~5 writes and a full parent + # re-roll-up per property (see TaskOrchestrator.run_subtasks). + orchestrator.run_subtasks( + task_id, + [{"property_id": pid} for pid in property_ids], + work=_work, + ) # Persist the whole batch in one transaction, then re-establish every # written Property's Baseline (the orchestrator batches its own UoW). The diff --git a/applications/modelling_e2e/modelling_e2e_trigger_body.py b/applications/modelling_e2e/modelling_e2e_trigger_body.py index f9c92513d..a234ca06c 100644 --- a/applications/modelling_e2e/modelling_e2e_trigger_body.py +++ b/applications/modelling_e2e/modelling_e2e_trigger_body.py @@ -1,3 +1,5 @@ +from typing import Optional + from pydantic import BaseModel, ConfigDict @@ -11,3 +13,8 @@ class ModellingE2ETriggerBody(BaseModel): refetch_epc: bool = True repredict_epc: bool = True dry_run: bool = False + # Attach mode (ADR-0055): a Modelling Run batch carries the app-owned task + # and the distributor's pre-created sub_task; the handler then creates no + # per-property child sub_tasks. Absent on classic (script) messages. + task_id: Optional[str] = None + subtask_id: Optional[str] = None From 3cf44d64099d587e06ad2a9c50c20847191853b9 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:55:49 +0000 Subject: [PATCH 38/56] =?UTF-8?q?An=20attach-mode=20partial=20failure=20pe?= =?UTF-8?q?rsists=20successes=20then=20records=20the=20failure=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../modelling_e2e/test_handler.py | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index fe7ef7815..f2e33187b 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -438,6 +438,79 @@ def test_attach_mode_models_the_batch_without_child_subtasks() -> None: assert [r.property_id for r in plan_requests] == [pid1, pid2] +def test_attach_mode_partial_failure_persists_successes_then_records_failure() -> None: + """In attach mode a failing property doesn't stop its siblings: the batch + flushes the successes, then raises SubTaskFailure carrying + {succeeded, failed} so the sub_task record holds the outcome (ADR-0055).""" + # Arrange — property 222's EPC fetch blows up; 111 models fine + pid_ok, pid_bad = 111, 222 + mock_engine = _engine_mock([pid_ok, pid_bad], [1001, 1002], [POSTCODE, POSTCODE]) + mock_orch = _mock_orchestrator() + task_id = uuid4() + + with ExitStack() as stack: + stack.enter_context(patch("applications.modelling_e2e.handler.os.environ", _ENV)) + stack.enter_context( + patch("applications.modelling_e2e.handler._get_engine", return_value=mock_engine) + ) + epc_client = stack.enter_context( + patch("applications.modelling_e2e.handler.EpcClientService") + ).return_value + epc_client.get_by_uprn.side_effect = [ + MagicMock(), + RuntimeError("gov API exploded"), + ] + stack.enter_context(patch("applications.modelling_e2e.handler.GeospatialS3Repository")) + stack.enter_context(patch("applications.modelling_e2e.handler.GoogleSolarApiClient")) + stack.enter_context( + patch("applications.modelling_e2e.handler._spatial_for", return_value=None) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler._solar_insights_for", return_value=None) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler.overlays_from", return_value=[]) + ) + stack.enter_context( + patch("applications.modelling_e2e.handler.PropertyOverridesPostgresReader") + ).return_value.overrides_for_many.return_value = {} + stack.enter_context( + patch("applications.modelling_e2e.handler.ScenarioPostgresRepository") + ).return_value.get_many.return_value = [MagicMock()] + stack.enter_context(patch("applications.modelling_e2e.handler.catalogue_snapshot_with_off_catalogue_overrides")) + stack.enter_context(patch("applications.modelling_e2e.handler.Session")) + stack.enter_context( + patch("applications.modelling_e2e.handler.run_modelling", return_value=_plan_mock()) + ) + MockUoW = stack.enter_context(patch("applications.modelling_e2e.handler.PostgresUnitOfWork")) + mock_uow = MagicMock() + MockUoW.return_value.__enter__.return_value = mock_uow + MockUoW.return_value.__exit__.return_value = False + + # Act + from applications.modelling_e2e.handler import handler + from domain.tasks.subtasks import SubTaskFailure + + with pytest.raises(SubTaskFailure) as raised: + handler.__wrapped__( # type: ignore[attr-defined] + {"task_id": str(task_id), "subtask_id": str(uuid4()), + "property_ids": [pid_ok, pid_bad], "portfolio_id": PORTFOLIO_ID, + "scenario_id": SCENARIO_ID, "refetch_solar": False, + "dry_run": False}, + None, mock_orch, task_id, + ) + + # Assert — the success flushed before the failure was recorded + plan_requests = mock_uow.plan.save_batch.call_args.args[0] + assert [r.property_id for r in plan_requests] == [pid_ok] + details = raised.value.details + assert details is not None + assert details["succeeded"] == 1 + assert details["failed"] == [ + {"property_id": pid_bad, "error": "gov API exploded"} + ] + + # --------------------------------------------------------------------------- # Lodged EPC path # --------------------------------------------------------------------------- From bf0d630a8feaaf05c79eed2edc58c022c96c5a87 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:56:57 +0000 Subject: [PATCH 39/56] =?UTF-8?q?An=20attach-mode=20partial=20failure=20pe?= =?UTF-8?q?rsists=20successes=20then=20records=20the=20failure=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- applications/modelling_e2e/handler.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index 9215a078c..b580a37d4 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -65,7 +65,7 @@ from domain.modelling.plan import Plan from domain.property.property import Property, PropertyIdentity from domain.property_baseline.calculator_rebaseliner import CalculatorRebaseliner from domain.sap10_calculator.calculator import Sap10Calculator -from domain.tasks.subtasks import SubTask +from domain.tasks.subtasks import SubTask, SubTaskFailure from domain.tasks.tasks import Source from harness.console import run_modelling from orchestration.task_orchestrator import TaskOrchestrator @@ -687,12 +687,21 @@ def handler( ) logger.info(f"property={pid} queued for write") + failed_properties: list[dict[str, Any]] = [] if trigger.subtask_id is not None: # Attach mode (ADR-0055): the whole batch runs under the # distributor's pre-created sub_task — the @task_handler wrapper is # already inside it — so no per-property children are created. + # Failures stay isolated per property (siblings continue) and are + # recorded on the sub_task after the surviving writes flush. for pid in property_ids: - _model_property(pid) + try: + _model_property(pid) + except Exception as exc: # noqa: BLE001 — recorded below + logger.exception(f"property={pid} failed") + failed_properties.append( + {"property_id": pid, "error": str(exc)} + ) else: # Fan the batch out into one child SubTask per property and run # them in a single batched pass: create all children, model each @@ -732,5 +741,18 @@ def handler( f"{[s['certificate_number'] for s in skipped_certs]}" ) + # Raised only after the surviving writes flushed: the failure is a + # record on the sub_task, never an SQS retry (ADR-0055). Recovery is a + # deliberate re-send of the sub_task's own inputs. + if failed_properties: + raise SubTaskFailure( + f"{len(failed_properties)} of {len(property_ids)} " + "properties failed", + details={ + "succeeded": len(property_ids) - len(failed_properties), + "failed": failed_properties, + }, + ) + finally: read_session.close() From 80c474b37bbc7a548a30fd577ba9e5141ba4fbc0 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:58:41 +0000 Subject: [PATCH 40/56] =?UTF-8?q?Batches=20never=20split=20a=20postcode=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/batching.py | 19 +++++++++++++ tests/backend/app/modelling/test_batching.py | 30 ++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 backend/app/modelling/batching.py create mode 100644 tests/backend/app/modelling/test_batching.py diff --git a/backend/app/modelling/batching.py b/backend/app/modelling/batching.py new file mode 100644 index 000000000..914746e24 --- /dev/null +++ b/backend/app/modelling/batching.py @@ -0,0 +1,19 @@ +"""Pack a Modelling Run's resolved properties into SQS-message-sized batches. + +Batches stay postcode-grouped — properties sharing a postcode ride the same +message so the workers' prediction cohort cache keeps paying (ADR-0055), the +same packing the trigger script has been running. +""" + +from backend.app.modelling.property_filters import FilteredProperty + +BATCH_SIZE = 50 + + +def pack_postcode_batches( + properties: list[FilteredProperty], batch_size: int = BATCH_SIZE +) -> list[list[FilteredProperty]]: + """Pack *properties* into batches of ~*batch_size*, never splitting a + postcode across batches. A single postcode larger than *batch_size* + becomes its own oversized batch.""" + raise NotImplementedError diff --git a/tests/backend/app/modelling/test_batching.py b/tests/backend/app/modelling/test_batching.py new file mode 100644 index 000000000..a7f196036 --- /dev/null +++ b/tests/backend/app/modelling/test_batching.py @@ -0,0 +1,30 @@ +from backend.app.modelling.batching import pack_postcode_batches +from backend.app.modelling.property_filters import FilteredProperty + + +def _properties(postcode: str, count: int, start_id: int) -> list[FilteredProperty]: + return [ + FilteredProperty(property_id=start_id + i, postcode=postcode) + for i in range(count) + ] + + +def test_postcodes_are_never_split_across_batches() -> None: + # arrange — 30 + 30 + 10 with a cap of 50: the second postcode won't fit + # alongside the first, the third rides with the second + properties = ( + _properties("B93 8SU", 30, start_id=0) + + _properties("M20 4TF", 30, start_id=100) + + _properties("SW1A 1AA", 10, start_id=200) + ) + + # act + batches = pack_postcode_batches(properties, batch_size=50) + + # assert + assert [len(b) for b in batches] == [30, 40] + for batch in batches: + for postcode in {p.postcode for p in batch}: + in_batch = [p for p in batch if p.postcode == postcode] + everywhere = [p for p in properties if p.postcode == postcode] + assert len(in_batch) == len(everywhere) # whole postcode, one batch From 2dd948259f6d2c00cd2db97894655f224cfa8e6b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 11:59:07 +0000 Subject: [PATCH 41/56] =?UTF-8?q?Batches=20never=20split=20a=20postcode=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/batching.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/backend/app/modelling/batching.py b/backend/app/modelling/batching.py index 914746e24..7cf02afef 100644 --- a/backend/app/modelling/batching.py +++ b/backend/app/modelling/batching.py @@ -16,4 +16,18 @@ def pack_postcode_batches( """Pack *properties* into batches of ~*batch_size*, never splitting a postcode across batches. A single postcode larger than *batch_size* becomes its own oversized batch.""" - raise NotImplementedError + groups: dict[str, list[FilteredProperty]] = {} + for prop in properties: + groups.setdefault(prop.postcode or "", []).append(prop) + + batches: list[list[FilteredProperty]] = [] + current: list[FilteredProperty] = [] + for group in groups.values(): + if current and len(current) + len(group) > batch_size: + batches.append(current) + current = list(group) + else: + current.extend(group) + if current: + batches.append(current) + return batches From 760648033874e976eba40eafcdc29c85c751d1ce Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:00:07 +0000 Subject: [PATCH 42/56] =?UTF-8?q?An=20oversized=20postcode=20becomes=20its?= =?UTF-8?q?=20own=20batch=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior emerged from the packing rule — pinned straight to green. Co-Authored-By: Claude Fable 5 --- tests/backend/app/modelling/test_batching.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/backend/app/modelling/test_batching.py b/tests/backend/app/modelling/test_batching.py index a7f196036..635f80e0b 100644 --- a/tests/backend/app/modelling/test_batching.py +++ b/tests/backend/app/modelling/test_batching.py @@ -28,3 +28,16 @@ def test_postcodes_are_never_split_across_batches() -> None: in_batch = [p for p in batch if p.postcode == postcode] everywhere = [p for p in properties if p.postcode == postcode] assert len(in_batch) == len(everywhere) # whole postcode, one batch + + +def test_an_oversized_postcode_becomes_its_own_batch() -> None: + # arrange — one postcode alone exceeds the cap; neighbours are unaffected + properties = _properties("B93 8SU", 60, start_id=0) + _properties( + "M20 4TF", 20, start_id=100 + ) + + # act + batches = pack_postcode_batches(properties, batch_size=50) + + # assert + assert [len(b) for b in batches] == [60, 20] From 489e2b5d47f9ff393f21a17b4dc5f22981060e76 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:02:12 +0000 Subject: [PATCH 43/56] =?UTF-8?q?Trigger-run=20fans=20out=20one=20sub=5Fta?= =?UTF-8?q?sk=20and=20message=20per=20scenario=20batch=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/config.py | 1 + backend/app/modelling/router.py | 73 ++++++++++ backend/app/modelling/schemas.py | 18 +++ .../app/modelling/test_trigger_run_router.py | 133 ++++++++++++++++++ 4 files changed, 225 insertions(+) create mode 100644 backend/app/modelling/router.py create mode 100644 backend/app/modelling/schemas.py create mode 100644 tests/backend/app/modelling/test_trigger_run_router.py diff --git a/backend/app/config.py b/backend/app/config.py index 8939e6ff8..d1c5f3ace 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -44,6 +44,7 @@ class Settings(BaseSettings): COMBINER_SQS_URL: str = "changeme" LANDLORD_OVERRIDES_SQS_URL: str = "changeme" FINALISER_SQS_URL: str = "changeme" + MODELLING_E2E_SQS_URL: str = "changeme" # Third parties EPC_AUTH_TOKEN: str = "changeme" diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py new file mode 100644 index 000000000..7eff93340 --- /dev/null +++ b/backend/app/modelling/router.py @@ -0,0 +1,73 @@ +"""The Modelling Run Distributor: POST /v1/modelling/trigger-run (ADR-0055). + +Accepts a portfolio-scoped modelling request expressed as filters, resolves +them to a concrete property set (ADR-0056), pre-creates one batch sub_task per +SQS message under the app-owned task, and fans the batches out to the +modelling_e2e workers. Never models synchronously; owns nothing after the +fan-out — progress and terminal state roll up from the workers. +""" + +import json +from collections.abc import Callable, Iterator +from typing import Any, cast + +import boto3 +from fastapi import APIRouter, Depends +from sqlmodel import Session + +from backend.app.config import get_settings +from backend.app.db.connection import db_engine +from backend.app.dependencies import validate_token +from backend.app.modelling.schemas import TriggerRunRequest + +# Sends pre-serialised message bodies to the modelling_e2e queue. A seam so +# tests record bodies instead of calling AWS. +MessageSender = Callable[[list[str]], None] + + +def get_session() -> Iterator[Session]: + with Session(db_engine) as session: + yield session + + +def get_message_sender() -> MessageSender: + settings = get_settings() + client: Any = cast( + Any, boto3.client("sqs", settings.AWS_DEFAULT_REGION) + ) # pyright: ignore[reportUnknownMemberType] + queue_url = settings.MODELLING_E2E_SQS_URL + + def send(bodies: list[str]) -> None: + # send_message_batch caps at 10 entries per call — chunk accordingly. + for start in range(0, len(bodies), 10): + chunk = bodies[start : start + 10] + client.send_message_batch( + QueueUrl=queue_url, + Entries=[ + {"Id": str(index), "MessageBody": body} + for index, body in enumerate(chunk) + ], + ) + + return send + + +router = APIRouter( + prefix="/modelling", + tags=["modelling"], + dependencies=[Depends(validate_token)], +) + + +@router.post("/trigger-run", status_code=202) +async def trigger_run( + body: TriggerRunRequest, + session: Session = Depends(get_session), + send_messages: MessageSender = Depends(get_message_sender), +) -> dict[str, str]: + raise NotImplementedError + + +# json is used by the implementation; referenced here so the stub imports are +# stable for the first RED. +_ = json diff --git a/backend/app/modelling/schemas.py b/backend/app/modelling/schemas.py new file mode 100644 index 000000000..272f5ab4d --- /dev/null +++ b/backend/app/modelling/schemas.py @@ -0,0 +1,18 @@ +"""Request contract for the Modelling Run Distributor (ADR-0055/0056).""" + +from uuid import UUID + +from pydantic import BaseModel + +from backend.app.modelling.property_filters import PropertyGroupFilters + + +class TriggerRunRequest(BaseModel): + """Exactly what the app sends: the app-created task, the portfolio scope, + the scenarios to model, and the property-group filters ({} = everything). + Anything else the distributor might want is readable from the task row.""" + + task_id: UUID + portfolio_id: int + scenario_ids: list[int] + filters: PropertyGroupFilters = PropertyGroupFilters() diff --git a/tests/backend/app/modelling/test_trigger_run_router.py b/tests/backend/app/modelling/test_trigger_run_router.py new file mode 100644 index 000000000..ae9e020be --- /dev/null +++ b/tests/backend/app/modelling/test_trigger_run_router.py @@ -0,0 +1,133 @@ +"""Tests for the Modelling Run Distributor endpoint (ADR-0055). + +The router's session and SQS seams are dependency-injected; tests run against +the ephemeral Postgres and record message bodies instead of calling AWS. +""" + +import json +from collections.abc import Iterator +from dataclasses import dataclass, field +from typing import Any +from uuid import uuid4 + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import Engine +from sqlmodel import Session + +from backend.app.dependencies import validate_token +from backend.app.modelling.router import get_message_sender, get_session, router +from domain.modelling.portfolio_goal import PortfolioGoal +from domain.tasks.tasks import Task +from infrastructure.postgres.modelling import ScenarioModel +from infrastructure.postgres.property_table import PropertyRow +from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository +from repositories.tasks.task_postgres_repository import TaskPostgresRepository + +PORTFOLIO_ID = 814 + + +@dataclass +class Api: + client: TestClient + engine: Engine + sent_bodies: list[str] = field(default_factory=list) + + def seed_task(self) -> Task: + with Session(self.engine) as session: + return TaskPostgresRepository(session).create( + Task.create(task_source="app:modelling_run", service="modelling_run") + ) + + def seed_scenario(self, *, portfolio_id: int = PORTFOLIO_ID) -> int: + with Session(self.engine) as session: + row = ScenarioModel( + portfolio_id=portfolio_id, + goal=PortfolioGoal.INCREASING_EPC, + goal_value="C", + ) + session.add(row) + session.commit() + session.refresh(row) + assert row.id is not None + return row.id + + def seed_property(self, *, postcode: str = "B93 8SU") -> int: + with Session(self.engine) as session: + row = PropertyRow(portfolio_id=PORTFOLIO_ID, postcode=postcode) + session.add(row) + session.commit() + session.refresh(row) + assert row.id is not None + return row.id + + def subtasks_for(self, task: Task) -> list[Any]: + with Session(self.engine) as session: + return list(SubTaskPostgresRepository(session).list_by_task(task.id)) + + +@pytest.fixture +def api(db_engine: Engine) -> Api: + app = FastAPI() + app.include_router(router, prefix="/v1") + harness = Api(client=TestClient(app), engine=db_engine) + + def _session() -> Iterator[Session]: + with Session(db_engine) as session: + yield session + + app.dependency_overrides[get_session] = _session + app.dependency_overrides[get_message_sender] = lambda: harness.sent_bodies.extend + app.dependency_overrides[validate_token] = lambda: "test-token" + return harness + + +def test_trigger_run_fans_out_one_subtask_and_message_per_scenario_batch( + api: Api, +) -> None: + # arrange — 3 properties (one batch) × 2 scenarios = 2 messages + task = api.seed_task() + scenario_a = api.seed_scenario() + scenario_b = api.seed_scenario() + property_ids = [api.seed_property() for _ in range(3)] + + # act + response = api.client.post( + "/v1/modelling/trigger-run", + json={ + "task_id": str(task.id), + "portfolio_id": PORTFOLIO_ID, + "scenario_ids": [scenario_a, scenario_b], + "filters": {}, + }, + ) + + # assert + assert response.status_code == 202 + subtasks = api.subtasks_for(task) + assert len(subtasks) == 2 + assert {s.status.value for s in subtasks} == {"waiting"} + + messages = [json.loads(body) for body in api.sent_bodies] + assert len(messages) == 2 + assert {m["scenario_id"] for m in messages} == {scenario_a, scenario_b} + for message in messages: + assert message["task_id"] == str(task.id) + assert message["portfolio_id"] == PORTFOLIO_ID + assert message["property_ids"] == property_ids + # ADR-0055 pinned flags: live fetch, live predict, solar only-if-missing + assert message["refetch_epc"] is True + assert message["repredict_epc"] is True + assert message["refetch_solar"] is True + assert message["dry_run"] is False + + # each message's subtask_id is one of the pre-created sub_tasks, and the + # sub_task's inputs are the message payload (its re-run recipe) + subtask_ids = {str(s.id) for s in subtasks} + assert {m["subtask_id"] for m in messages} == subtask_ids + by_id = {str(s.id): s for s in subtasks} + for message in messages: + inputs = by_id[message["subtask_id"]].inputs + assert inputs["property_ids"] == message["property_ids"] + assert inputs["scenario_id"] == message["scenario_id"] From 9031c16ba76b7a4b23afa4d2806451865978088e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:03:33 +0000 Subject: [PATCH 44/56] =?UTF-8?q?Trigger-run=20fans=20out=20one=20sub=5Fta?= =?UTF-8?q?sk=20and=20message=20per=20scenario=20batch=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/router.py | 49 ++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py index 7eff93340..71e74ae4f 100644 --- a/backend/app/modelling/router.py +++ b/backend/app/modelling/router.py @@ -18,7 +18,14 @@ from sqlmodel import Session from backend.app.config import get_settings from backend.app.db.connection import db_engine from backend.app.dependencies import validate_token +from backend.app.modelling.batching import pack_postcode_batches +from backend.app.modelling.property_filters import ( + FilteredProperty, + resolve_filtered_property_ids, +) from backend.app.modelling.schemas import TriggerRunRequest +from domain.tasks.subtasks import SubTask +from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository # Sends pre-serialised message bodies to the modelling_e2e queue. A seam so # tests record bodies instead of calling AWS. @@ -32,9 +39,7 @@ def get_session() -> Iterator[Session]: def get_message_sender() -> MessageSender: settings = get_settings() - client: Any = cast( - Any, boto3.client("sqs", settings.AWS_DEFAULT_REGION) - ) # pyright: ignore[reportUnknownMemberType] + client: Any = cast(Any, boto3.client("sqs", settings.AWS_DEFAULT_REGION)) # pyright: ignore[reportUnknownMemberType] queue_url = settings.MODELLING_E2E_SQS_URL def send(bodies: list[str]) -> None: @@ -65,9 +70,39 @@ async def trigger_run( session: Session = Depends(get_session), send_messages: MessageSender = Depends(get_message_sender), ) -> dict[str, str]: - raise NotImplementedError + properties: list[FilteredProperty] = resolve_filtered_property_ids( + session, body.portfolio_id, body.filters + ) + batches = pack_postcode_batches(properties) + # Pre-create one sub_task per (scenario, batch) message under the + # app-owned task, each holding its exact message payload — the fixed + # progress denominator and the batch's re-run recipe (ADR-0055). + subtask_repo = SubTaskPostgresRepository(session) + subtasks: list[SubTask] = [] + payloads: list[dict[str, Any]] = [] + for scenario_id in body.scenario_ids: + for batch in batches: + payload: dict[str, Any] = { + "task_id": str(body.task_id), + "property_ids": [p.property_id for p in batch], + "portfolio_id": body.portfolio_id, + "scenario_id": scenario_id, + # ADR-0055 pinned flags: live EPC fetch, live prediction, + # solar fetched only where no stored row exists. + "refetch_epc": True, + "repredict_epc": True, + "refetch_solar": True, + "dry_run": False, + } + subtasks.append(SubTask.create(task_id=body.task_id, inputs=payload)) + payloads.append(payload) + subtask_repo.create_many(subtasks) -# json is used by the implementation; referenced here so the stub imports are -# stable for the first RED. -_ = json + send_messages( + [ + json.dumps({**payload, "subtask_id": str(subtask.id)}) + for payload, subtask in zip(payloads, subtasks) + ] + ) + return {"message": "Modelling Run distributed"} From dade7a3a5ff60c98b0e4e891d980daf550dd825e Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:04:05 +0000 Subject: [PATCH 45/56] =?UTF-8?q?A=20task=20that=20already=20has=20sub=5Ft?= =?UTF-8?q?asks=20refuses=20re-distribution=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_trigger_run_router.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/backend/app/modelling/test_trigger_run_router.py b/tests/backend/app/modelling/test_trigger_run_router.py index ae9e020be..d383fae1d 100644 --- a/tests/backend/app/modelling/test_trigger_run_router.py +++ b/tests/backend/app/modelling/test_trigger_run_router.py @@ -83,6 +83,37 @@ def api(db_engine: Engine) -> Api: return harness +def _trigger(api: Api, task: Task, scenario_ids: list[int]) -> Any: + return api.client.post( + "/v1/modelling/trigger-run", + json={ + "task_id": str(task.id), + "portfolio_id": PORTFOLIO_ID, + "scenario_ids": scenario_ids, + "filters": {}, + }, + ) + + +def test_trigger_run_rejects_a_task_that_already_has_subtasks(api: Api) -> None: + """A blind retry or double-submit must not double the fan-out (ADR-0055): + the app checks the task's progress instead of re-POSTing.""" + # arrange — a task that has already been distributed + task = api.seed_task() + scenario = api.seed_scenario() + api.seed_property() + assert _trigger(api, task, [scenario]).status_code == 202 + already_sent = len(api.sent_bodies) + + # act + response = _trigger(api, task, [scenario]) + + # assert — refused, nothing new created or sent + assert response.status_code == 409 + assert len(api.sent_bodies) == already_sent + assert len(api.subtasks_for(task)) == 1 + + def test_trigger_run_fans_out_one_subtask_and_message_per_scenario_batch( api: Api, ) -> None: From cfedb8e9cf032fc5ba0f1e2c8f0bb239d51af64d Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:04:34 +0000 Subject: [PATCH 46/56] =?UTF-8?q?A=20task=20that=20already=20has=20sub=5Ft?= =?UTF-8?q?asks=20refuses=20re-distribution=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/router.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py index 71e74ae4f..6be284523 100644 --- a/backend/app/modelling/router.py +++ b/backend/app/modelling/router.py @@ -12,7 +12,7 @@ from collections.abc import Callable, Iterator from typing import Any, cast import boto3 -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException from sqlmodel import Session from backend.app.config import get_settings @@ -70,6 +70,16 @@ async def trigger_run( session: Session = Depends(get_session), send_messages: MessageSender = Depends(get_message_sender), ) -> dict[str, str]: + subtask_repo = SubTaskPostgresRepository(session) + if subtask_repo.list_by_task(body.task_id): + raise HTTPException( + status_code=409, + detail=( + f"Task {body.task_id} already has sub_tasks — it has been " + "distributed. Check its progress instead of re-triggering." + ), + ) + properties: list[FilteredProperty] = resolve_filtered_property_ids( session, body.portfolio_id, body.filters ) @@ -78,7 +88,6 @@ async def trigger_run( # Pre-create one sub_task per (scenario, batch) message under the # app-owned task, each holding its exact message payload — the fixed # progress denominator and the batch's re-run recipe (ADR-0055). - subtask_repo = SubTaskPostgresRepository(session) subtasks: list[SubTask] = [] payloads: list[dict[str, Any]] = [] for scenario_id in body.scenario_ids: From e9b39df3226a14005071496dbe501835936325f7 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:05:03 +0000 Subject: [PATCH 47/56] =?UTF-8?q?Filters=20that=20match=20no=20properties?= =?UTF-8?q?=20refuse=20the=20run=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_trigger_run_router.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/backend/app/modelling/test_trigger_run_router.py b/tests/backend/app/modelling/test_trigger_run_router.py index d383fae1d..9da1405f2 100644 --- a/tests/backend/app/modelling/test_trigger_run_router.py +++ b/tests/backend/app/modelling/test_trigger_run_router.py @@ -114,6 +114,23 @@ def test_trigger_run_rejects_a_task_that_already_has_subtasks(api: Api) -> None: assert len(api.subtasks_for(task)) == 1 +def test_trigger_run_rejects_filters_that_match_no_properties(api: Api) -> None: + """Zero resolved properties is a caller error (ADR-0055): a task with no + sub_tasks could never roll up to complete, and the app's preview shows the + same zero from the same rule before POSTing.""" + # arrange — a portfolio with no properties at all + task = api.seed_task() + scenario = api.seed_scenario() + + # act + response = _trigger(api, task, [scenario]) + + # assert + assert response.status_code == 400 + assert api.sent_bodies == [] + assert api.subtasks_for(task) == [] + + def test_trigger_run_fans_out_one_subtask_and_message_per_scenario_batch( api: Api, ) -> None: From e9eb068be9ad47ed7ece7a7a38d47ece7da7326b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:05:39 +0000 Subject: [PATCH 48/56] =?UTF-8?q?Filters=20that=20match=20no=20properties?= =?UTF-8?q?=20refuse=20the=20run=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/router.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py index 6be284523..d71c699f6 100644 --- a/backend/app/modelling/router.py +++ b/backend/app/modelling/router.py @@ -83,6 +83,16 @@ async def trigger_run( properties: list[FilteredProperty] = resolve_filtered_property_ids( session, body.portfolio_id, body.filters ) + if not properties: + # A task with zero sub_tasks could never roll up to complete; the + # app's preview shows the same zero from the same rule (ADR-0056). + raise HTTPException( + status_code=400, + detail=( + f"The filters resolve to no properties in portfolio " + f"{body.portfolio_id} — nothing to distribute." + ), + ) batches = pack_postcode_batches(properties) # Pre-create one sub_task per (scenario, batch) message under the From ca799cc5e5181faa743dffefe45dd742503a666b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:06:02 +0000 Subject: [PATCH 49/56] =?UTF-8?q?Scenarios=20outside=20the=20portfolio=20r?= =?UTF-8?q?efuse=20the=20run=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_trigger_run_router.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/backend/app/modelling/test_trigger_run_router.py b/tests/backend/app/modelling/test_trigger_run_router.py index 9da1405f2..30ebb17b7 100644 --- a/tests/backend/app/modelling/test_trigger_run_router.py +++ b/tests/backend/app/modelling/test_trigger_run_router.py @@ -131,6 +131,22 @@ def test_trigger_run_rejects_filters_that_match_no_properties(api: Api) -> None: assert api.subtasks_for(task) == [] +def test_trigger_run_rejects_scenarios_outside_the_portfolio(api: Api) -> None: + # arrange — one valid scenario, one belonging to a different portfolio + task = api.seed_task() + ours = api.seed_scenario() + theirs = api.seed_scenario(portfolio_id=999) + api.seed_property() + + # act + response = _trigger(api, task, [ours, theirs]) + + # assert — refused outright; nothing partially distributed + assert response.status_code == 400 + assert api.sent_bodies == [] + assert api.subtasks_for(task) == [] + + def test_trigger_run_fans_out_one_subtask_and_message_per_scenario_batch( api: Api, ) -> None: From b22b9cbca0907278772a43a5aa81157f22e5f9fb Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:06:42 +0000 Subject: [PATCH 50/56] =?UTF-8?q?Scenarios=20outside=20the=20portfolio=20r?= =?UTF-8?q?efuse=20the=20run=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/router.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py index d71c699f6..09a9f5bb3 100644 --- a/backend/app/modelling/router.py +++ b/backend/app/modelling/router.py @@ -13,7 +13,7 @@ from typing import Any, cast import boto3 from fastapi import APIRouter, Depends, HTTPException -from sqlmodel import Session +from sqlmodel import Session, col, select from backend.app.config import get_settings from backend.app.db.connection import db_engine @@ -25,6 +25,7 @@ from backend.app.modelling.property_filters import ( ) from backend.app.modelling.schemas import TriggerRunRequest from domain.tasks.subtasks import SubTask +from infrastructure.postgres.modelling import ScenarioModel from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository # Sends pre-serialised message bodies to the modelling_e2e queue. A seam so @@ -80,6 +81,24 @@ async def trigger_run( ), ) + scenario_rows = session.exec( + select(ScenarioModel).where(col(ScenarioModel.id).in_(body.scenario_ids)) + ).all() + portfolio_by_scenario = {row.id: row.portfolio_id for row in scenario_rows} + invalid = [ + scenario_id + for scenario_id in body.scenario_ids + if portfolio_by_scenario.get(scenario_id) != body.portfolio_id + ] + if invalid: + raise HTTPException( + status_code=400, + detail=( + f"Scenarios {invalid} do not belong to portfolio " + f"{body.portfolio_id}." + ), + ) + properties: list[FilteredProperty] = resolve_filtered_property_ids( session, body.portfolio_id, body.filters ) From b62901a9de673c6c1e2fc93194fd4a01403f9f04 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:15:31 +0000 Subject: [PATCH 51/56] =?UTF-8?q?The=20distributor=20coexists=20with=20bot?= =?UTF-8?q?h=20table=20stacks=20and=20mounts=20at=20/v1/modelling=20?= =?UTF-8?q?=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DB access via parameterised SQL, not the infrastructure SQLModel mirrors: importing those alongside the legacy backend.app.db.models mirrors of the same tables double-registers them in the shared metadata and crashes the app at import. Contained to the modelling package until the DDD cut-over. Co-Authored-By: Claude Fable 5 --- backend/app/main.py | 2 + backend/app/modelling/property_filters.py | 142 ++++++++++++---------- backend/app/modelling/router.py | 86 ++++++++----- 3 files changed, 138 insertions(+), 92 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index 0dd9e489b..4b4aaaa70 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -11,6 +11,7 @@ from backend.app.whlg import router as whlg_router from backend.app.plan import router as plan_router from backend.app.tasks import router as tasks_router from backend.app.bulk_uploads import router as bulk_uploads_router +from backend.app.modelling import router as modelling_router from backend.app.dependencies import validate_api_key from backend.app.config import get_settings @@ -63,6 +64,7 @@ app.include_router(portfolio_router.router, prefix="/v1") app.include_router(plan_router.router, prefix="/v1") app.include_router(whlg_router.router, prefix="/v1") app.include_router(bulk_uploads_router.router, prefix="/v1") +app.include_router(modelling_router.router, prefix="/v1") if get_settings().ENVIRONMENT == "local": from backend.app.local import router as local_router diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index ce398cb25..0c0934fb0 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -4,18 +4,21 @@ The resolution rule is a shared contract with the app's preview (ADR-0056): the "N properties will be modelled" count the user approves is computed by the Next.js app from this same rule, so any change here must land in both codebases and amend the ADR. + +Reads go through parameterised raw SQL rather than the SQLModel table mirrors: +the FastAPI app registers the legacy ``backend.app.db.models`` mirrors of +``property``/``sub_task``, and importing the ``infrastructure.postgres`` +mirrors of the same tables into one process double-registers them in the +shared SQLModel metadata and crashes the app at import. Until the DDD +cut-over unifies the stacks, this module stays model-free. """ -from collections.abc import Callable from dataclasses import dataclass -from typing import Optional +from typing import Any, Optional from pydantic import BaseModel, ConfigDict -from sqlmodel import Session, col, select - -from infrastructure.postgres.epc_property_table import EpcPropertyModel -from infrastructure.postgres.property_override_table import PropertyOverrideRow -from infrastructure.postgres.property_table import PropertyRow +from sqlalchemy import text +from sqlmodel import Session class PropertyGroupFilters(BaseModel): @@ -48,13 +51,13 @@ _UNKNOWN = "Unknown" class _ComponentResolution: """How one filterable component (property_type / built_form) resolves at each ADR-0056 precedence level: which override row names it, how its RdSAP - numeric codes map (text labels pass through as-is), and where it lives on - the EPC and the legacy property row.""" + numeric codes map (text labels pass through as-is), and which columns hold + it on the EPC and the legacy property row.""" override_component: str codes: dict[str, str] - epc_value: Callable[[EpcPropertyModel], Optional[str]] - legacy_value: Callable[[PropertyRow], Optional[str]] + epc_columns: tuple[str, ...] + legacy_column: str _PROPERTY_TYPE = _ComponentResolution( @@ -67,8 +70,8 @@ _PROPERTY_TYPE = _ComponentResolution( "4": "Park home", }, # A cert without the property_type column falls back to its dwelling_type. - epc_value=lambda epc: epc.property_type or epc.dwelling_type, - legacy_value=lambda row: row.property_type, + epc_columns=("property_type", "dwelling_type"), + legacy_column="property_type", ) _BUILT_FORM = _ComponentResolution( @@ -81,8 +84,8 @@ _BUILT_FORM = _ComponentResolution( "5": "Enclosed End-Terrace", "6": "Enclosed Mid-Terrace", }, - epc_value=lambda epc: epc.built_form, - legacy_value=lambda row: row.built_form, + epc_columns=("built_form",), + legacy_column="built_form", ) @@ -93,17 +96,29 @@ def resolve_filtered_property_ids( portfolio's rows with ``marked_for_deletion = false``; property_type / built_form resolve override → lodged EPC → predicted EPC → legacy columns → "Unknown". Filters AND-combine; an absent key is unconstrained.""" - statement = ( - select(PropertyRow) - .where(PropertyRow.portfolio_id == portfolio_id) - # IS NOT TRUE rather than == False: the FE schema defaults the flag, - # but a NULL must never silently exclude a property. - .where(col(PropertyRow.marked_for_deletion).isnot(True)) - .order_by(col(PropertyRow.id)) + # IS NOT TRUE rather than = false: the FE schema defaults the flag, but a + # NULL must never silently exclude a property. + base_sql = ( + "SELECT id, postcode, property_type, built_form FROM property" + " WHERE portfolio_id = :portfolio_id" + " AND marked_for_deletion IS NOT TRUE" ) + params: dict[str, Any] = {"portfolio_id": portfolio_id} if filters.postcodes is not None: - statement = statement.where(col(PropertyRow.postcode).in_(filters.postcodes)) - candidates = [row for row in session.exec(statement).all() if row.id is not None] + base_sql += " AND postcode = ANY(:postcodes)" + params["postcodes"] = filters.postcodes + base_sql += " ORDER BY id" + candidates = [ + _Candidate( + property_id=int(row[0]), + postcode=row[1], + legacy_values={ + _PROPERTY_TYPE.legacy_column: row[2], + _BUILT_FORM.legacy_column: row[3], + }, + ) + for row in session.connection().execute(text(base_sql), params) + ] for wanted_values, component in ( (filters.property_types, _PROPERTY_TYPE), @@ -114,60 +129,65 @@ def resolve_filtered_property_ids( wanted = set(wanted_values) resolved = _resolved_component_values(session, candidates, component) candidates = [ - row - for row in candidates - if row.id is not None and resolved.get(row.id) in wanted + c for c in candidates if resolved.get(c.property_id) in wanted ] return [ - FilteredProperty(property_id=row.id, postcode=row.postcode) - for row in candidates - if row.id is not None + FilteredProperty(property_id=c.property_id, postcode=c.postcode) + for c in candidates ] +@dataclass(frozen=True) +class _Candidate: + property_id: int + postcode: Optional[str] + legacy_values: dict[str, Optional[str]] + + def _resolved_component_values( - session: Session, candidates: list[PropertyRow], component: _ComponentResolution + session: Session, candidates: list[_Candidate], component: _ComponentResolution ) -> dict[int, str]: """Each candidate's effective value for *component* per the ADR-0056 precedence: override (building_part 0) → lodged EPC → predicted EPC → legacy column → "Unknown".""" - property_ids = [row.id for row in candidates if row.id is not None] + property_ids = [c.property_id for c in candidates] - overrides = session.exec( - select(PropertyOverrideRow) - .where(col(PropertyOverrideRow.property_id).in_(property_ids)) - .where(PropertyOverrideRow.building_part == 0) - .where(PropertyOverrideRow.override_component == component.override_component) - ).all() - by_override = {o.property_id: o.override_value for o in overrides} + override_rows = session.connection().execute( + text( + "SELECT property_id, override_value FROM property_overrides" + " WHERE property_id = ANY(:ids)" + " AND building_part = 0" + " AND override_component = :component" + ), + {"ids": property_ids, "component": component.override_component}, + ) + by_override = {int(row[0]): str(row[1]) for row in override_rows} - epcs = session.exec( - select(EpcPropertyModel) - .where(col(EpcPropertyModel.property_id).in_(property_ids)) - .where(col(EpcPropertyModel.source).in_(("lodged", "predicted"))) - ).all() + epc_columns = ", ".join(component.epc_columns) + epc_rows = session.connection().execute( + text( + f"SELECT property_id, source, {epc_columns} FROM epc_property" + " WHERE property_id = ANY(:ids)" + " AND source IN ('lodged', 'predicted')" + ), + {"ids": property_ids}, + ) by_epc_source: dict[str, dict[int, str]] = {"lodged": {}, "predicted": {}} - for epc in epcs: - value = component.epc_value(epc) - if epc.property_id is not None and value: - by_epc_source[epc.source][epc.property_id] = component.codes.get( - value, value + for row in epc_rows: + value = next((v for v in row[2:] if v), None) + if value is not None: + by_epc_source[str(row[1])][int(row[0])] = component.codes.get( + str(value), str(value) ) - by_legacy_column = { - row.id: value - for row in candidates - if row.id is not None and (value := component.legacy_value(row)) is not None - } - return { - pid: ( - by_override.get(pid) - or by_epc_source["lodged"].get(pid) - or by_epc_source["predicted"].get(pid) - or by_legacy_column.get(pid) + c.property_id: ( + by_override.get(c.property_id) + or by_epc_source["lodged"].get(c.property_id) + or by_epc_source["predicted"].get(c.property_id) + or c.legacy_values.get(component.legacy_column) or _UNKNOWN ) - for pid in property_ids + for c in candidates } diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py index 09a9f5bb3..dcb8fbebf 100644 --- a/backend/app/modelling/router.py +++ b/backend/app/modelling/router.py @@ -9,11 +9,14 @@ fan-out — progress and terminal state roll up from the workers. import json from collections.abc import Callable, Iterator +from datetime import datetime, timezone from typing import Any, cast +from uuid import uuid4 import boto3 from fastapi import APIRouter, Depends, HTTPException -from sqlmodel import Session, col, select +from sqlalchemy import text +from sqlmodel import Session from backend.app.config import get_settings from backend.app.db.connection import db_engine @@ -24,9 +27,12 @@ from backend.app.modelling.property_filters import ( resolve_filtered_property_ids, ) from backend.app.modelling.schemas import TriggerRunRequest -from domain.tasks.subtasks import SubTask -from infrastructure.postgres.modelling import ScenarioModel -from repositories.tasks.subtask_postgres_repository import SubTaskPostgresRepository + +# DB access is parameterised raw SQL, not the SQLModel mirrors: the FastAPI +# app registers the legacy backend.app.db.models mirrors of these tables, and +# importing the infrastructure.postgres mirrors of the same tables into one +# process double-registers them and crashes the app at import (see +# property_filters — same containment, until the DDD cut-over). # Sends pre-serialised message bodies to the modelling_e2e queue. A seam so # tests record bodies instead of calling AWS. @@ -71,8 +77,11 @@ async def trigger_run( session: Session = Depends(get_session), send_messages: MessageSender = Depends(get_message_sender), ) -> dict[str, str]: - subtask_repo = SubTaskPostgresRepository(session) - if subtask_repo.list_by_task(body.task_id): + already_distributed = session.connection().execute( + text("SELECT 1 FROM sub_task WHERE task_id = :task_id LIMIT 1"), + {"task_id": body.task_id}, + ).first() + if already_distributed is not None: raise HTTPException( status_code=409, detail=( @@ -81,10 +90,11 @@ async def trigger_run( ), ) - scenario_rows = session.exec( - select(ScenarioModel).where(col(ScenarioModel.id).in_(body.scenario_ids)) - ).all() - portfolio_by_scenario = {row.id: row.portfolio_id for row in scenario_rows} + scenario_rows = session.connection().execute( + text("SELECT id, portfolio_id FROM scenario WHERE id = ANY(:ids)"), + {"ids": body.scenario_ids}, + ) + portfolio_by_scenario = {int(row[0]): row[1] for row in scenario_rows} invalid = [ scenario_id for scenario_id in body.scenario_ids @@ -117,30 +127,44 @@ async def trigger_run( # Pre-create one sub_task per (scenario, batch) message under the # app-owned task, each holding its exact message payload — the fixed # progress denominator and the batch's re-run recipe (ADR-0055). - subtasks: list[SubTask] = [] - payloads: list[dict[str, Any]] = [] + messages: list[dict[str, Any]] = [] for scenario_id in body.scenario_ids: for batch in batches: - payload: dict[str, Any] = { - "task_id": str(body.task_id), - "property_ids": [p.property_id for p in batch], - "portfolio_id": body.portfolio_id, - "scenario_id": scenario_id, - # ADR-0055 pinned flags: live EPC fetch, live prediction, - # solar fetched only where no stored row exists. - "refetch_epc": True, - "repredict_epc": True, - "refetch_solar": True, - "dry_run": False, - } - subtasks.append(SubTask.create(task_id=body.task_id, inputs=payload)) - payloads.append(payload) - subtask_repo.create_many(subtasks) + messages.append( + { + "task_id": str(body.task_id), + "subtask_id": str(uuid4()), + "property_ids": [p.property_id for p in batch], + "portfolio_id": body.portfolio_id, + "scenario_id": scenario_id, + # ADR-0055 pinned flags: live EPC fetch, live prediction, + # solar fetched only where no stored row exists. + "refetch_epc": True, + "repredict_epc": True, + "refetch_solar": True, + "dry_run": False, + } + ) - send_messages( + now = datetime.now(timezone.utc) + session.connection().execute( + text( + "INSERT INTO sub_task (id, task_id, status, inputs, updated_at)" + " VALUES (:id, :task_id, 'waiting', :inputs, :updated_at)" + ), [ - json.dumps({**payload, "subtask_id": str(subtask.id)}) - for payload, subtask in zip(payloads, subtasks) - ] + { + "id": message["subtask_id"], + "task_id": body.task_id, + "inputs": json.dumps( + {k: v for k, v in message.items() if k != "subtask_id"} + ), + "updated_at": now, + } + for message in messages + ], ) + session.commit() + + send_messages([json.dumps(message) for message in messages]) return {"message": "Modelling Run distributed"} From bf8647c9b551eda439896211a74b8c31a876feaf Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:16:42 +0000 Subject: [PATCH 52/56] =?UTF-8?q?The=20409=20guard=20and=20batch=20creatio?= =?UTF-8?q?n=20read=20as=20methods=20on=20ModellingRunTasks=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/modelling/router.py | 35 +++--------------- backend/app/modelling/run_tasks.py | 58 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 31 deletions(-) create mode 100644 backend/app/modelling/run_tasks.py diff --git a/backend/app/modelling/router.py b/backend/app/modelling/router.py index dcb8fbebf..168973392 100644 --- a/backend/app/modelling/router.py +++ b/backend/app/modelling/router.py @@ -9,7 +9,6 @@ fan-out — progress and terminal state roll up from the workers. import json from collections.abc import Callable, Iterator -from datetime import datetime, timezone from typing import Any, cast from uuid import uuid4 @@ -26,14 +25,9 @@ from backend.app.modelling.property_filters import ( FilteredProperty, resolve_filtered_property_ids, ) +from backend.app.modelling.run_tasks import ModellingRunTasks from backend.app.modelling.schemas import TriggerRunRequest -# DB access is parameterised raw SQL, not the SQLModel mirrors: the FastAPI -# app registers the legacy backend.app.db.models mirrors of these tables, and -# importing the infrastructure.postgres mirrors of the same tables into one -# process double-registers them and crashes the app at import (see -# property_filters — same containment, until the DDD cut-over). - # Sends pre-serialised message bodies to the modelling_e2e queue. A seam so # tests record bodies instead of calling AWS. MessageSender = Callable[[list[str]], None] @@ -77,11 +71,8 @@ async def trigger_run( session: Session = Depends(get_session), send_messages: MessageSender = Depends(get_message_sender), ) -> dict[str, str]: - already_distributed = session.connection().execute( - text("SELECT 1 FROM sub_task WHERE task_id = :task_id LIMIT 1"), - {"task_id": body.task_id}, - ).first() - if already_distributed is not None: + run_tasks = ModellingRunTasks(session) + if run_tasks.already_distributed(body.task_id): raise HTTPException( status_code=409, detail=( @@ -146,25 +137,7 @@ async def trigger_run( } ) - now = datetime.now(timezone.utc) - session.connection().execute( - text( - "INSERT INTO sub_task (id, task_id, status, inputs, updated_at)" - " VALUES (:id, :task_id, 'waiting', :inputs, :updated_at)" - ), - [ - { - "id": message["subtask_id"], - "task_id": body.task_id, - "inputs": json.dumps( - {k: v for k, v in message.items() if k != "subtask_id"} - ), - "updated_at": now, - } - for message in messages - ], - ) - session.commit() + run_tasks.create_batch_subtasks(body.task_id, messages) send_messages([json.dumps(message) for message in messages]) return {"message": "Modelling Run distributed"} diff --git a/backend/app/modelling/run_tasks.py b/backend/app/modelling/run_tasks.py new file mode 100644 index 000000000..7d7021b04 --- /dev/null +++ b/backend/app/modelling/run_tasks.py @@ -0,0 +1,58 @@ +"""The distributor's view of the app-owned task's sub_tasks (ADR-0055). + +Intention-revealing methods over parameterised SQL. SQL rather than the +SQLModel mirrors because the FastAPI app registers the legacy +``backend.app.db.models`` mirrors of ``sub_task``, and importing the +``infrastructure.postgres`` mirror of the same table into one process +double-registers it and crashes the app at import (contained here until the +DDD cut-over). +""" + +import json +from datetime import datetime, timezone +from typing import Any +from uuid import UUID + +from sqlalchemy import text +from sqlmodel import Session + + +class ModellingRunTasks: + def __init__(self, session: Session) -> None: + self._session = session + + def already_distributed(self, task_id: UUID) -> bool: + """Whether the task already has sub_tasks — i.e. a distributor has + already fanned it out (the 409 guard against double-submits).""" + row = self._session.connection().execute( + text("SELECT 1 FROM sub_task WHERE task_id = :task_id LIMIT 1"), + {"task_id": task_id}, + ).first() + return row is not None + + def create_batch_subtasks( + self, task_id: UUID, messages: list[dict[str, Any]] + ) -> None: + """Pre-create one ``waiting`` sub_task per batch message under the + app-owned task. Each sub_task's inputs are its exact message payload + (minus the self-referential subtask_id) — the fixed progress + denominator and the batch's re-run recipe.""" + now = datetime.now(timezone.utc) + self._session.connection().execute( + text( + "INSERT INTO sub_task (id, task_id, status, inputs, updated_at)" + " VALUES (:id, :task_id, 'waiting', :inputs, :updated_at)" + ), + [ + { + "id": message["subtask_id"], + "task_id": task_id, + "inputs": json.dumps( + {k: v for k, v in message.items() if k != "subtask_id"} + ), + "updated_at": now, + } + for message in messages + ], + ) + self._session.commit() From 60c448e66dd833c9bbf1b7e4db2f477ff6b0271a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:18:48 +0000 Subject: [PATCH 53/56] FastAPI Lambda can send to the modelling_e2e queue Remote state + MODELLING_E2E_SQS_URL env var + send policy ARN, and fast_api_lambda deploys after modelling_e2e_lambda so the remote state exists before it applies. Co-Authored-By: Claude Fable 5 --- .github/workflows/deploy_terraform.yml | 2 +- deployment/terraform/lambda/fast-api/main.tf | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy_terraform.yml b/.github/workflows/deploy_terraform.yml index 5e469dc66..0f72a33f1 100644 --- a/.github/workflows/deploy_terraform.yml +++ b/.github/workflows/deploy_terraform.yml @@ -538,7 +538,7 @@ jobs: # Deploy FastAPI Lambda # ============================================================ fast_api_lambda: - needs: [determine_stage, ara_engine_lambda, categorisation_lambda, postcodeSplitter_lambda, bulk_address2uprn_combiner_lambda, bulkUploadFinaliser_lambda] + needs: [determine_stage, ara_engine_lambda, categorisation_lambda, postcodeSplitter_lambda, bulk_address2uprn_combiner_lambda, bulkUploadFinaliser_lambda, modelling_e2e_lambda] uses: ./.github/workflows/_deploy_lambda.yml with: lambda_name: ara_fast_api diff --git a/deployment/terraform/lambda/fast-api/main.tf b/deployment/terraform/lambda/fast-api/main.tf index 696f46650..26e154d6a 100644 --- a/deployment/terraform/lambda/fast-api/main.tf +++ b/deployment/terraform/lambda/fast-api/main.tf @@ -64,6 +64,15 @@ data "terraform_remote_state" "landlord_description_overrides" { } } +data "terraform_remote_state" "modelling_e2e" { + backend = "s3" + config = { + bucket = "modelling-e2e-terraform-state", + key = "env:/${var.stage}/terraform.tfstate" + region = "eu-west-2" + } +} + ############################################ # Load Credentials ############################################ @@ -126,6 +135,7 @@ module "fastapi" { COMBINER_SQS_URL = data.terraform_remote_state.bulk_address2uprn_combiner.outputs.bulk_address2uprn_combiner_queue_url FINALISER_SQS_URL = data.terraform_remote_state.bulk_upload_finaliser.outputs.bulk_upload_finaliser_queue_url LANDLORD_OVERRIDES_SQS_URL = data.terraform_remote_state.landlord_description_overrides.outputs.landlord_description_overrides_queue_url + MODELLING_E2E_SQS_URL = data.terraform_remote_state.modelling_e2e.outputs.modelling_e2e_queue_url } } @@ -149,7 +159,8 @@ module "fastapi_sqs_policy" { data.terraform_remote_state.postcode_splitter.outputs.postcode_splitter_queue_arn, data.terraform_remote_state.bulk_address2uprn_combiner.outputs.bulk_address2uprn_combiner_queue_arn, data.terraform_remote_state.bulk_upload_finaliser.outputs.bulk_upload_finaliser_queue_arn, - data.terraform_remote_state.landlord_description_overrides.outputs.landlord_description_overrides_queue_arn + data.terraform_remote_state.landlord_description_overrides.outputs.landlord_description_overrides_queue_arn, + data.terraform_remote_state.modelling_e2e.outputs.modelling_e2e_queue_arn ] conditions = null From ddd64a92975f72306e83c8cc7a60703080788d2b Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:49:19 +0000 Subject: [PATCH 54/56] =?UTF-8?q?Legacy=20property=20columns=20no=20longer?= =?UTF-8?q?=20resolve=20type=20or=20built=20form=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../app/modelling/test_property_filters.py | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/tests/backend/app/modelling/test_property_filters.py b/tests/backend/app/modelling/test_property_filters.py index b7d029368..dda558547 100644 --- a/tests/backend/app/modelling/test_property_filters.py +++ b/tests/backend/app/modelling/test_property_filters.py @@ -1,6 +1,6 @@ from typing import Optional -from sqlalchemy import Engine +from sqlalchemy import Engine, text from sqlmodel import Session from backend.app.modelling.property_filters import ( @@ -220,24 +220,39 @@ def test_epc_without_property_type_falls_back_to_dwelling_type( assert [p.property_id for p in resolved] == [fallback] -def test_property_without_epc_falls_back_to_the_legacy_column( - db_engine: Engine, -) -> None: - # arrange — no override, no EPC rows; only property.property_type +def test_legacy_property_columns_are_ignored(db_engine: Engine) -> None: + """property.property_type / built_form are legacy: with no override and no + EPC the property is "Unknown", whatever the legacy columns say (ADR-0056 + amendment).""" + # arrange — no override, no EPC rows; the FE-owned legacy column exists in + # prod but is no longer mirrored, so add it here and set a value that must + # not count with Session(db_engine) as session: legacy = _seed_property(session, portfolio_id=814) - session.get_one(PropertyRow, legacy).property_type = "Bungalow" + session.connection().execute( + text("ALTER TABLE property ADD COLUMN IF NOT EXISTS property_type TEXT") + ) + session.connection().execute( + text("UPDATE property SET property_type = 'Bungalow' WHERE id = :id"), + {"id": legacy}, + ) session.commit() # act - resolved = resolve_filtered_property_ids( + as_bungalow = resolve_filtered_property_ids( session, portfolio_id=814, filters=PropertyGroupFilters(property_types=["Bungalow"]), ) + as_unknown = resolve_filtered_property_ids( + session, + portfolio_id=814, + filters=PropertyGroupFilters(property_types=["Unknown"]), + ) # assert - assert [p.property_id for p in resolved] == [legacy] + assert as_bungalow == [] + assert [p.property_id for p in as_unknown] == [legacy] def test_unknown_is_a_selectable_property_type_bucket(db_engine: Engine) -> None: From 07190fc332aad025e83b06152b48888b1ee400d8 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 12:52:11 +0000 Subject: [PATCH 55/56] =?UTF-8?q?Legacy=20property=20columns=20no=20longer?= =?UTF-8?q?=20resolve=20type=20or=20built=20form=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0056 amended: override and EPC own type/form facts; a property with neither is Unknown whatever the legacy columns say. Mirror columns removed. Co-Authored-By: Claude Fable 5 --- backend/app/modelling/property_filters.py | 25 ++++++------------- ...-a-shared-contract-with-the-app-preview.md | 12 ++++++--- infrastructure/postgres/property_table.py | 9 +++---- 3 files changed, 19 insertions(+), 27 deletions(-) diff --git a/backend/app/modelling/property_filters.py b/backend/app/modelling/property_filters.py index 0c0934fb0..44d8f19eb 100644 --- a/backend/app/modelling/property_filters.py +++ b/backend/app/modelling/property_filters.py @@ -52,12 +52,12 @@ class _ComponentResolution: """How one filterable component (property_type / built_form) resolves at each ADR-0056 precedence level: which override row names it, how its RdSAP numeric codes map (text labels pass through as-is), and which columns hold - it on the EPC and the legacy property row.""" + it on the EPC. The legacy property.property_type / built_form columns are + NOT consulted (ADR-0056 amendment — overrides own those facts).""" override_component: str codes: dict[str, str] epc_columns: tuple[str, ...] - legacy_column: str _PROPERTY_TYPE = _ComponentResolution( @@ -71,7 +71,6 @@ _PROPERTY_TYPE = _ComponentResolution( }, # A cert without the property_type column falls back to its dwelling_type. epc_columns=("property_type", "dwelling_type"), - legacy_column="property_type", ) _BUILT_FORM = _ComponentResolution( @@ -85,7 +84,6 @@ _BUILT_FORM = _ComponentResolution( "6": "Enclosed Mid-Terrace", }, epc_columns=("built_form",), - legacy_column="built_form", ) @@ -94,12 +92,12 @@ def resolve_filtered_property_ids( ) -> list[FilteredProperty]: """Resolve *filters* against *portfolio_id* per ADR-0056: base set is the portfolio's rows with ``marked_for_deletion = false``; property_type / - built_form resolve override → lodged EPC → predicted EPC → legacy columns - → "Unknown". Filters AND-combine; an absent key is unconstrained.""" + built_form resolve override → lodged EPC → predicted EPC → "Unknown". + Filters AND-combine; an absent key is unconstrained.""" # IS NOT TRUE rather than = false: the FE schema defaults the flag, but a # NULL must never silently exclude a property. base_sql = ( - "SELECT id, postcode, property_type, built_form FROM property" + "SELECT id, postcode FROM property" " WHERE portfolio_id = :portfolio_id" " AND marked_for_deletion IS NOT TRUE" ) @@ -109,14 +107,7 @@ def resolve_filtered_property_ids( params["postcodes"] = filters.postcodes base_sql += " ORDER BY id" candidates = [ - _Candidate( - property_id=int(row[0]), - postcode=row[1], - legacy_values={ - _PROPERTY_TYPE.legacy_column: row[2], - _BUILT_FORM.legacy_column: row[3], - }, - ) + _Candidate(property_id=int(row[0]), postcode=row[1]) for row in session.connection().execute(text(base_sql), params) ] @@ -142,7 +133,6 @@ def resolve_filtered_property_ids( class _Candidate: property_id: int postcode: Optional[str] - legacy_values: dict[str, Optional[str]] def _resolved_component_values( @@ -150,7 +140,7 @@ def _resolved_component_values( ) -> dict[int, str]: """Each candidate's effective value for *component* per the ADR-0056 precedence: override (building_part 0) → lodged EPC → predicted EPC → - legacy column → "Unknown".""" + "Unknown".""" property_ids = [c.property_id for c in candidates] override_rows = session.connection().execute( @@ -186,7 +176,6 @@ def _resolved_component_values( by_override.get(c.property_id) or by_epc_source["lodged"].get(c.property_id) or by_epc_source["predicted"].get(c.property_id) - or c.legacy_values.get(component.legacy_column) or _UNKNOWN ) for c in candidates diff --git a/docs/adr/0056-modelling-run-filter-resolution-is-a-shared-contract-with-the-app-preview.md b/docs/adr/0056-modelling-run-filter-resolution-is-a-shared-contract-with-the-app-preview.md index 2dd8f56e8..9e13a5732 100644 --- a/docs/adr/0056-modelling-run-filter-resolution-is-a-shared-contract-with-the-app-preview.md +++ b/docs/adr/0056-modelling-run-filter-resolution-is-a-shared-contract-with-the-app-preview.md @@ -40,9 +40,7 @@ lands in both codebases and amends this ADR. built_form: 1=Detached, 2=Semi-Detached, 3=End-Terrace, 4=Mid-Terrace, 5=Enclosed End-Terrace, 6=Enclosed Mid-Terrace. Text passes through as-is; - 3. else the legacy `property.property_type` / `property.built_form` - columns; - 4. else **"Unknown"** — a selectable filter value meaning exactly this + 3. else **"Unknown"** — a selectable filter value meaning exactly this bucket (no resolvable value at any level). 4. An absent filter key is unconstrained; present keys combine with **AND**. @@ -57,3 +55,11 @@ lands in both codebases and amends this ADR. - Drift risk is accepted and mitigated by this document; a cross-repo contract test (same fixture set, both implementations) is the natural follow-on if drift ever bites. + +## Amendment (2026-07-08): the legacy property columns are not consulted + +The original rule fell back to `property.property_type` / `property.built_form` +before "Unknown". Those columns are legacy: the override layer owns +user-supplied type/form facts, so a property with no override and no EPC is +**"Unknown"** whatever the legacy columns say. The app's preview must apply +the same amendment. diff --git a/infrastructure/postgres/property_table.py b/infrastructure/postgres/property_table.py index c3cf497c2..e62ac958e 100644 --- a/infrastructure/postgres/property_table.py +++ b/infrastructure/postgres/property_table.py @@ -57,10 +57,7 @@ class PropertyRow(SQLModel, table=True): has_recommendations: Optional[bool] = Field(default=None) updated_at: Optional[datetime] = Field(default=None) - # Columns the Modelling Run filter resolution reads (ADR-0056): the base-set - # exclusion flag and the legacy type/form columns (the last resort before - # "Unknown" in the resolution precedence). NOT NULL / defaulted in the FE - # schema; nullable mirrors are safe — prod enforces the real constraints. + # The Modelling Run filter resolution's base-set exclusion flag (ADR-0056). + # The legacy property_type / built_form columns are deliberately NOT + # mirrored: overrides and EPCs own those facts (ADR-0056 amendment). marked_for_deletion: Optional[bool] = Field(default=None) - property_type: Optional[str] = Field(default=None) - built_form: Optional[str] = Field(default=None) From cc4bf4394e872ee8176d275dc2902ef12dc9a161 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Tue, 7 Jul 2026 13:24:02 +0000 Subject: [PATCH 56/56] =?UTF-8?q?Both=20postcode=20batchers=20share=20one?= =?UTF-8?q?=20group-preserving=20packing=20core=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback (#1481): the address batcher and the Modelling Run batcher implemented the same greedy packing; the core moves to utilities/grouped_batching.py and both become thin wrappers. Co-Authored-By: Claude Fable 5 --- backend/app/modelling/batching.py | 21 ++++-------- domain/addresses/postcode_batching.py | 44 +++--------------------- utilities/grouped_batching.py | 48 +++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 54 deletions(-) create mode 100644 utilities/grouped_batching.py diff --git a/backend/app/modelling/batching.py b/backend/app/modelling/batching.py index 7cf02afef..785855c41 100644 --- a/backend/app/modelling/batching.py +++ b/backend/app/modelling/batching.py @@ -6,6 +6,7 @@ same packing the trigger script has been running. """ from backend.app.modelling.property_filters import FilteredProperty +from utilities.grouped_batching import iter_grouped_batches BATCH_SIZE = 50 @@ -16,18 +17,8 @@ def pack_postcode_batches( """Pack *properties* into batches of ~*batch_size*, never splitting a postcode across batches. A single postcode larger than *batch_size* becomes its own oversized batch.""" - groups: dict[str, list[FilteredProperty]] = {} - for prop in properties: - groups.setdefault(prop.postcode or "", []).append(prop) - - batches: list[list[FilteredProperty]] = [] - current: list[FilteredProperty] = [] - for group in groups.values(): - if current and len(current) + len(group) > batch_size: - batches.append(current) - current = list(group) - else: - current.extend(group) - if current: - batches.append(current) - return batches + return list( + iter_grouped_batches( + properties, key=lambda p: p.postcode or "", max_batch_size=batch_size + ) + ) diff --git a/domain/addresses/postcode_batching.py b/domain/addresses/postcode_batching.py index ca4cd7529..7de6a5b43 100644 --- a/domain/addresses/postcode_batching.py +++ b/domain/addresses/postcode_batching.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections.abc import Iterable, Iterator from domain.addresses.unstandardised_address import AddressList, UnstandardisedAddress -from domain.postcode import Postcode +from utilities.grouped_batching import iter_grouped_batches def iter_postcode_grouped_batches( @@ -11,41 +11,7 @@ def iter_postcode_grouped_batches( *, max_batch_size: int = 500, ) -> Iterator[AddressList]: - if max_batch_size < 1: - raise ValueError("max_batch_size must be >= 1") - - groups = _group_by_postcode_in_order(addresses) - - buffer: AddressList = AddressList([]) - for group in groups.values(): - group_len = len(group) - - # Oversize single-Postcode group: flush buffer first, then dispatch - # the group as its own batch. Mirrors the legacy - # ``if group_len >= batch_size`` branch. - if group_len >= max_batch_size: - if buffer: - yield buffer - buffer = AddressList([]) - yield group - continue - - # Adding this group would overflow: flush buffer before appending. - if len(buffer) + group_len > max_batch_size: - yield buffer - buffer = AddressList([]) - - buffer.extend(group) - - # Final flush. - if buffer: - yield buffer - - -def _group_by_postcode_in_order( - addresses: Iterable[UnstandardisedAddress], -) -> dict[Postcode, AddressList]: - groups: dict[Postcode, AddressList] = {} - for address in addresses: - groups.setdefault(address.postcode, AddressList([])).append(address) - return groups + for batch in iter_grouped_batches( + addresses, key=lambda a: a.postcode, max_batch_size=max_batch_size + ): + yield AddressList(batch) diff --git a/utilities/grouped_batching.py b/utilities/grouped_batching.py new file mode 100644 index 000000000..b5e42b3f9 --- /dev/null +++ b/utilities/grouped_batching.py @@ -0,0 +1,48 @@ +"""Greedy group-preserving batching. + +Packs items into batches of at most ``max_batch_size`` without ever splitting +a group (items sharing a key) across batches; a single group larger than the +cap becomes its own oversized batch. Shared by the address postcode batcher +(``domain/addresses/postcode_batching.py``) and the Modelling Run distributor +(``backend/app/modelling/batching.py``). +""" + +from collections.abc import Callable, Hashable, Iterable, Iterator +from typing import TypeVar + +T = TypeVar("T") + + +def iter_grouped_batches( + items: Iterable[T], + *, + key: Callable[[T], Hashable], + max_batch_size: int, +) -> Iterator[list[T]]: + if max_batch_size < 1: + raise ValueError("max_batch_size must be >= 1") + + groups: dict[Hashable, list[T]] = {} + for item in items: + groups.setdefault(key(item), []).append(item) + + buffer: list[T] = [] + for group in groups.values(): + # Oversize single-key group: flush the buffer first, then dispatch + # the group as its own batch. + if len(group) >= max_batch_size: + if buffer: + yield buffer + buffer = [] + yield group + continue + + # Adding this group would overflow: flush the buffer before appending. + if len(buffer) + len(group) > max_batch_size: + yield buffer + buffer = [] + + buffer.extend(group) + + if buffer: + yield buffer