From e1bc5360f2698df0fb983949ad4a35225ef8ab4f Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Wed, 24 Jun 2026 11:11:19 +0000 Subject: [PATCH] =?UTF-8?q?skipped=20cohort=20certs=20are=20logged=20witho?= =?UTF-8?q?ut=20failing=20the=20coordinator=20SubTask=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- applications/modelling_e2e/handler.py | 5 ----- .../modelling_e2e/test_handler.py | 21 ++++++------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/applications/modelling_e2e/handler.py b/applications/modelling_e2e/handler.py index a715c7bf..32c76d7d 100644 --- a/applications/modelling_e2e/handler.py +++ b/applications/modelling_e2e/handler.py @@ -25,7 +25,6 @@ from __future__ import annotations import dataclasses import io -import json import os from collections.abc import Callable from typing import Any, Optional, cast @@ -460,9 +459,5 @@ def handler(body: dict[str, Any], context: Any, orchestrator: TaskOrchestrator, f"{[s['certificate_number'] for s in skipped_certs]}" ) - if skipped_certs: - raise RuntimeError( - f"skipped_unmappable_cohort_certs: {json.dumps(skipped_certs)}" - ) finally: read_session.close() diff --git a/tests/applications/modelling_e2e/test_handler.py b/tests/applications/modelling_e2e/test_handler.py index 2a01e258..e2664a4b 100644 --- a/tests/applications/modelling_e2e/test_handler.py +++ b/tests/applications/modelling_e2e/test_handler.py @@ -293,12 +293,9 @@ def test_lodged_epc_path_saves_epc_plan_and_marks_modelled( _baseline_orchestrator.return_value.run.assert_called_once_with([PROPERTY_ID]) -def test_skipped_cohort_certs_fail_the_subtask_but_the_plan_is_still_saved() -> None: - """Cohort certs the mapper can't consume are skipped (so prediction is not - aborted), then surfaced as a failure — the subtask is marked failed (the - cert numbers land in outputs.error via the raised RuntimeError) so the - mapper gaps get debugged. The batch still ran to completion first, so the - property's plan was committed before the handler raised.""" +def test_skipped_cohort_certs_do_not_prevent_plan_being_saved() -> None: + """Cohort certs the mapper can't consume are skipped so prediction can + proceed — the property's plan is committed before the handler returns.""" from repositories.comparable_properties.epc_comparable_properties_repository import ( SkippedCohortCert, ) @@ -372,16 +369,10 @@ def test_skipped_cohort_certs_fail_the_subtask_but_the_plan_is_still_saved() -> MockUoW.return_value.__enter__.return_value = mock_uow MockUoW.return_value.__exit__.return_value = False - # Act — the skipped cert fails the subtask, but only after the batch ran. - with pytest.raises(RuntimeError) as excinfo: - _call_handler(_BODY) + # Act — no raise even with a skipped cert + _call_handler(_BODY) - # Assert — the cert number + error reach outputs.error (the raised message), - # and the property's plan was still committed before the handler raised. - message = str(excinfo.value) - assert "skipped_unmappable_cohort_certs" in message - assert "8257-7539-1649-0633-4992" in message - assert "RdSapSchema17_1: missing required field 'window'" in message + # Assert — plan committed despite the skipped cert mock_uow.plan.save.assert_called_once() mock_uow.commit.assert_called_once()