From 62e1d4b813b18b915cb86c2bcb74296b6e0b9441 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 8 Jun 2026 20:35:45 +0000 Subject: [PATCH] fix(product): deterministic catalogue pick by ordering get() by id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProductPostgresRepository.get took .first() with no ORDER BY, so when a measure type has several active material rows (the live catalogue holds 74 solar_pv, 5 high_heat_retention_storage_heaters) the chosen row — hence the cost and material_id — depended on the database's physical row order. Order by id so a re-seed prices the same product every time. Co-Authored-By: Claude Opus 4.8 --- .../product/product_postgres_repository.py | 7 +++- .../test_product_postgres_repository.py | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/repositories/product/product_postgres_repository.py b/repositories/product/product_postgres_repository.py index 5a46348c..7f57baa9 100644 --- a/repositories/product/product_postgres_repository.py +++ b/repositories/product/product_postgres_repository.py @@ -17,11 +17,16 @@ class ProductPostgresRepository(ProductRepository): self._session = session def get(self, measure_type: str) -> Product: + # The live catalogue holds many active rows per type; order by id so the + # pick is deterministic (a re-seed prices the same) rather than relying + # on the database's physical row order. row: MaterialRow | None = self._session.exec( - select(MaterialRow).where( + select(MaterialRow) + .where( col(MaterialRow.type) == measure_type, col(MaterialRow.is_active).is_(True), ) + .order_by(col(MaterialRow.id)) ).first() if row is None: raise ValueError(f"no active product for measure type {measure_type!r}") diff --git a/tests/repositories/product/test_product_postgres_repository.py b/tests/repositories/product/test_product_postgres_repository.py index 13293ea6..7a0c84ec 100644 --- a/tests/repositories/product/test_product_postgres_repository.py +++ b/tests/repositories/product/test_product_postgres_repository.py @@ -42,6 +42,43 @@ def test_get_maps_active_material_to_product_with_contingency( assert abs(product.contingency_rate - 0.10) <= 1e-9 +def test_get_picks_the_lowest_id_when_several_active_rows_share_a_type( + db_engine: Engine, +) -> None: + # Arrange — the live catalogue holds many active rows per type (e.g. 74 + # solar_pv); the choice must be deterministic so a re-run prices the same. + with Session(db_engine) as session: + session.add_all( + [ + MaterialRow( + id=7, + type="solar_pv", + total_cost=99.0, + cost_unit="gbp_per_unit", + is_active=True, + description="Solar PV (higher id)", + ), + MaterialRow( + id=3, + type="solar_pv", + total_cost=42.0, + cost_unit="gbp_per_unit", + is_active=True, + description="Solar PV (lowest id)", + ), + ] + ) + session.commit() + + # Act + with Session(db_engine) as session: + product: Product = ProductPostgresRepository(session).get("solar_pv") + + # Assert — the lowest-id active row wins, deterministically. + assert product.id == 3 + assert abs(product.unit_cost_per_m2 - 42.0) <= 1e-9 + + def test_get_raises_when_only_an_inactive_product_exists(db_engine: Engine) -> None: # Arrange with Session(db_engine) as session: