From c1a558d2b544fc1f30379075d1fb3206da141599 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Thu, 2 Jul 2026 14:19:31 +0000 Subject: [PATCH 1/2] =?UTF-8?q?refetch=5Fsolar=3DFalse=20models=20with=20s?= =?UTF-8?q?tored=20Solar=20insights=20and=20never=20calls=20Google=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 | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index a09fe266a..80c44aeeb 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -124,6 +124,18 @@ def _baseline_orchestrator() -> Iterator[MagicMock]: yield orchestrator +@pytest.fixture(autouse=True) +def _solar_repository() -> Iterator[MagicMock]: + """Patch the stored-Solar repository for every test — the handler reads + stored insights through it whatever refetch_solar says. Default: no stored + rows; solar tests override get_many with real payloads.""" + with patch( + "applications.modelling_e2e.handler.SolarPostgresRepository" + ) as repository: + repository.return_value.get_many.return_value = {} + yield repository + + # --------------------------------------------------------------------------- # Solar throttle: per-container call gap resolved from env (quota/N safety) # --------------------------------------------------------------------------- @@ -208,6 +220,96 @@ def test_trigger_body_new_flags_default_to_true() -> None: assert result.refetch_solar is True +# --------------------------------------------------------------------------- +# refetch_solar semantics — False skips the LIVE fetch only, never the measure +# --------------------------------------------------------------------------- + + +def test_refetch_solar_false_models_with_stored_solar_and_never_calls_google( + _solar_repository: MagicMock, +) -> None: + """refetch_solar=False means "don't pay for a Google call", NOT "model + without solar": the stored Solar insights are read from the DB and handed to + the Modelling stage, and the live Google client is never invoked. + + Provenance: portfolio 814's 2026-07-02 re-model ran with refetch_solar=False + and every plan lost its solar candidates (pids 742071/742077/742397/742399 — + houses stopped short of goal B with bill-negative ASHP-only plans) even + though the `solar` table held healthy payloads for their UPRNs.""" + # Arrange + stored_insights = {"solarPotential": {"maxArrayPanelsCount": 58}} + mock_engine = _engine_mock([PROPERTY_ID], [UPRN], [POSTCODE]) + mock_uow = MagicMock() + + 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) + ) + mock_live_fetch = 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")) + mock_run_modelling = stack.enter_context( + patch( + "applications.modelling_e2e.handler.run_modelling", + return_value=_plan_mock(), + ) + ) + MockUoW = stack.enter_context( + patch("applications.modelling_e2e.handler.PostgresUnitOfWork") + ) + MockUoW.return_value.__enter__.return_value = mock_uow + MockUoW.return_value.__exit__.return_value = False + _solar_repository.return_value.get_many.return_value = { + UPRN: stored_insights + } + + # Act — _BODY carries refetch_solar=False + _call_handler(_BODY) + + # Assert — the stored insights reach the Modelling stage unchanged, and the + # paid Google leg is never exercised. + assert ( + mock_run_modelling.call_args.kwargs["solar_insights"] == stored_insights + ) + mock_live_fetch.assert_not_called() + + # --------------------------------------------------------------------------- # Child SubTask creation # --------------------------------------------------------------------------- From 100949073f4324afd8f23e584cd3272a8d89ab95 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Thu, 2 Jul 2026 14:21:46 +0000 Subject: [PATCH 2/2] =?UTF-8?q?refetch=5Fsolar=3DFalse=20models=20with=20s?= =?UTF-8?q?tored=20Solar=20insights=20and=20never=20calls=20Google=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, 13 insertions(+), 13 deletions(-) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index b4848f694..bf9ca8b06 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -512,12 +512,11 @@ def handler( # held-open read Session. (The ``finally`` is the safety net.) scenario = ScenarioPostgresRepository(read_session).get_many([scenario_id])[0] products = catalogue_snapshot_with_off_catalogue_overrides(read_session) + # Stored Solar is read whatever refetch_solar says — the flag decides + # whether a UPRN with no stored row gets a live Google fetch, never + # whether solar is modelled at all. stored_solar: dict[int, Optional[dict[str, Any]]] = ( - {} - if not refetch_solar - else SolarPostgresRepository(read_session).get_many( - list(set(uprns.values())) - ) + SolarPostgresRepository(read_session).get_many(list(set(uprns.values()))) ) epc_repo = EpcPostgresRepository(read_session) stored_lodged_epcs: dict[int, EpcPropertyData] = ( @@ -615,15 +614,16 @@ def handler( landlord_overrides=overrides, ).effective_epc - solar_insights: Optional[dict[str, Any]] + # refetch_solar gates ONLY the paid Google call for UPRNs with no + # stored row; stored insights always feed the Modelling stage. + # (False here once meant "no solar at all" — the pre-rename + # `no_solar` semantics — which silently stripped solar from every + # plan in a re-model batch.) + solar_insights: Optional[dict[str, Any]] = stored_solar.get(uprn) solar_was_fetched = False - if not refetch_solar: - solar_insights = None - else: - solar_insights = stored_solar.get(uprn) - if solar_insights is None: - solar_insights = _solar_insights_for(solar_client, spatial) - solar_was_fetched = solar_insights is not None + if solar_insights is None and refetch_solar: + solar_insights = _solar_insights_for(solar_client, spatial) + solar_was_fetched = solar_insights is not None plan = run_modelling( effective_epc,