From 16185b207a1690dad22ba4a22f61e6a2fc38cf81 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Fri, 3 Jul 2026 19:18:51 +0000 Subject: [PATCH] fix: tolerate expert_op4grid_recommender's typed-pipeline return shape The upstream expert_op4grid_recommender typed-pipeline refactor (R1, 0.2.7) replaced run_analysis_step1's (res_step1, context) 2-tuple with a single AnalysisContext (proceed) or AnalysisResult (no-overload short-circuit). AnalysisMixin.run_analysis_step1 now normalises both shapes via _normalize_step1_outcome, so the backend keeps working against both the legacy and the refactored recommender releases (same version-tolerance pattern as _upstream_step1_supports_prebuilt_obs). The new AnalysisContext / AnalysisResult dataclasses expose a dict-compatible view, so every context[...] / result.get(...) access in the service layer is unchanged. Verified against the real refactored recommender: this fixes the two tests that drive the live step1 (test_step1_detects_single_overload, test_step2_returns_prioritized_action_set) with zero regressions across the backend suite. Also: annotate _normalize_step1_outcome's return type (backend code-quality ratchet: 61 -> 60), and add expert_backend/tests/test_step1_outcome_normalization.py covering the tuple / isinstance / structural-fallback paths and the service-level short-circuit-vs-proceed decision (real-dataclass cases skip under the mock layer). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01LGzwaP2SnQCbLKo4ZNuwSg Signed-off-by: Antoine Marot --- CHANGELOG.md | 13 ++ expert_backend/services/analysis_mixin.py | 38 +++- expert_backend/tests/CLAUDE.md | 1 + .../tests/test_step1_outcome_normalization.py | 162 ++++++++++++++++++ 4 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 expert_backend/tests/test_step1_outcome_normalization.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d1a7d..9e15dac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and the project (informally) follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Compatibility with the `expert_op4grid_recommender` typed-pipeline refactor + +- **`run_analysis_step1` now tolerates the library's new single-value return.** + Upstream `expert_op4grid_recommender` replaced the analysis pipeline's + `(res_step1, context)` 2-tuple with a single `AnalysisContext` (proceed) or + `AnalysisResult` (no-overload short-circuit). `AnalysisMixin.run_analysis_step1` + now normalises both shapes via `_normalize_step1_outcome`, so the backend + keeps working against both the legacy and the refactored recommender releases + (same version-tolerance pattern as `_upstream_step1_supports_prebuilt_obs`). + The `AnalysisContext` / `AnalysisResult` dataclasses keep a dict-compatible + view, so every `context[...]` / `result.get(...)` access in the service layer + is unchanged. + ### SLD readability & loading coherence on the PyPSA grids Three related fixes for the Single Line Diagram on the PyPSA-EUR grids, diff --git a/expert_backend/services/analysis_mixin.py b/expert_backend/services/analysis_mixin.py index 80e1db4..cc3a7a5 100644 --- a/expert_backend/services/analysis_mixin.py +++ b/expert_backend/services/analysis_mixin.py @@ -89,6 +89,40 @@ def _upstream_step1_supports_prebuilt_obs() -> bool: return False +def _normalize_step1_outcome(outcome: Any) -> tuple[Any, Any]: + """Normalise ``run_analysis_step1``'s return across recommender versions. + + Older ``expert_op4grid_recommender`` releases returned a + ``(res_step1, context)`` 2-tuple where a non-``None`` ``res_step1`` meant + "no actionable overload". Newer releases (the typed-pipeline refactor) + return a **single** value instead: an ``AnalysisResult`` (the no-overload + short-circuit) or an ``AnalysisContext`` (proceed to step 2). This + normaliser accepts either shape and always returns the legacy + ``(res_step1, context)`` pair so the rest of the wrapper is + version-agnostic — the same tolerance pattern as + ``_upstream_step1_supports_prebuilt_obs``. + """ + # Legacy contract: an explicit (res_step1, context) tuple. + if isinstance(outcome, tuple) and len(outcome) == 2: + return outcome + # Newer contract: a single AnalysisResult / AnalysisContext. Detect the + # short-circuit result by type when the class is importable, else probe + # structurally (a result exposes ``prioritized_actions`` and carries no + # live observation, whereas a context always carries ``obs``). + try: + from expert_op4grid_recommender.main import AnalysisResult + is_early_result = isinstance(outcome, AnalysisResult) + except (ImportError, TypeError): + is_early_result = ( + hasattr(outcome, "get") + and "obs" not in outcome + and outcome.get("prioritized_actions") is not None + ) + if is_early_result: + return outcome, None + return None, outcome + + if TYPE_CHECKING: from expert_backend.services._recommender_state import RecommenderState @@ -484,7 +518,9 @@ def run_analysis_step1(self, disconnected_elements) -> dict: "expert_op4grid_recommender lacks ``prebuilt_obs_simu_defaut`` " "support — falling back to the full contingency simulation." ) - res_step1, context = run_analysis_step1(**step1_kwargs) + res_step1, context = _normalize_step1_outcome( + run_analysis_step1(**step1_kwargs) + ) self._last_disconnected_elements = list(norm) # Stash the wall-clock so step 2 can forward it to the result # event — the React UI shows the full breakdown including diff --git a/expert_backend/tests/CLAUDE.md b/expert_backend/tests/CLAUDE.md index 044f746..5065554 100644 --- a/expert_backend/tests/CLAUDE.md +++ b/expert_backend/tests/CLAUDE.md @@ -53,6 +53,7 @@ Configuration in `frontend/vite.config.ts` (Vitest plugin). |------|-------------| | `test_recommender_simulation.py` | Real data simulation with small test grid | | `test_split_analysis.py` | Two-step analysis workflow (step1 overload detect, step2 resolve) | +| `test_step1_outcome_normalization.py` | `_normalize_step1_outcome` — tolerates the recommender's typed-pipeline return (single `AnalysisContext`/`AnalysisResult`) as well as the legacy `(res, context)` 2-tuple; tuple + isinstance + structural-fallback paths, plus the service short-circuit/proceed decision. Real-dataclass cases skip under the mock layer. | | `test_combined_actions_integration.py` | Combined action workflow integration (incl. PR #114 LS/curtailment in combined pairs) | | `test_combined_actions_scenario.py` | Real-world combined action scenarios | | `test_stream_pdf_integration.py` | Streaming NDJSON + PDF event integration | diff --git a/expert_backend/tests/test_step1_outcome_normalization.py b/expert_backend/tests/test_step1_outcome_normalization.py new file mode 100644 index 0000000..dba3865 --- /dev/null +++ b/expert_backend/tests/test_step1_outcome_normalization.py @@ -0,0 +1,162 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# SPDX-License-Identifier: MPL-2.0 +"""Coverage for ``_normalize_step1_outcome`` — cross-version tolerance of +``run_analysis_step1``'s return shape. + +Older ``expert_op4grid_recommender`` releases returned a ``(res_step1, +context)`` 2-tuple (non-``None`` ``res_step1`` = "no actionable overload"). +The typed-pipeline refactor returns a single ``AnalysisContext`` (proceed) or +``AnalysisResult`` (short-circuit). ``AnalysisMixin.run_analysis_step1`` must +keep working against BOTH, so it normalises the outcome back to the legacy +pair. These tests pin: + +* the tuple pass-through (always), +* the ``isinstance(outcome, AnalysisResult)`` path (real library installed), +* the structural fallback used when the class isn't importable (mock layer), +* and the service-level short-circuit / proceed decisions. + +They run in BOTH modes: under the ``conftest.py`` mock layer (library absent) +and against a real ``expert_op4grid_recommender`` install (CI). The tests that +require the real dataclasses skip cleanly when the library is mocked. +""" +from __future__ import annotations + +import sys +from unittest.mock import MagicMock + +import pytest + +from expert_op4grid_recommender import config +from expert_backend.services import analysis_mixin as _analysis_mixin +from expert_backend.services.analysis_mixin import _normalize_step1_outcome +from expert_backend.services.recommender_service import RecommenderService + +# Detect whether the REAL typed dataclasses are importable (CI) vs the mock +# layer (a MagicMock attribute is not a ``type``). +try: + from expert_op4grid_recommender.main import ( # noqa: F401 + AnalysisContext as _RealContext, + AnalysisResult as _RealResult, + ) + _HAS_REAL = isinstance(_RealResult, type) and isinstance(_RealContext, type) +except Exception: # pragma: no cover - defensive + _RealContext = _RealResult = None + _HAS_REAL = False + +_needs_real = pytest.mark.skipif( + not _HAS_REAL, + reason="real expert_op4grid_recommender typed dataclasses not installed", +) + + +# --------------------------------------------------------------------- +# Unit: _normalize_step1_outcome +# --------------------------------------------------------------------- + +class TestNormalizeStep1Outcome: + def test_legacy_tuple_passes_through(self): + assert _normalize_step1_outcome(("RESULT", "CONTEXT")) == ("RESULT", "CONTEXT") + + def test_legacy_tuple_short_circuit_shape(self): + # (result, None) — the historical "no overload" signal. + assert _normalize_step1_outcome(("R", None)) == ("R", None) + + def _force_structural_fallback(self, monkeypatch): + # Make ``AnalysisResult`` a non-type so ``isinstance`` raises TypeError, + # forcing the structural probe deterministically in any environment. + main_mod = sys.modules["expert_op4grid_recommender.main"] + monkeypatch.setattr(main_mod, "AnalysisResult", object(), raising=False) + + def test_structural_probe_detects_result(self, monkeypatch): + self._force_structural_fallback(monkeypatch) + # A result exposes prioritized_actions and carries no live obs. + result_like = {"lines_overloaded_names": ["L1"], "prioritized_actions": {}, + "action_scores": {}} + assert _normalize_step1_outcome(result_like) == (result_like, None) + + def test_structural_probe_detects_context(self, monkeypatch): + self._force_structural_fallback(monkeypatch) + context_like = {"obs": object(), "lines_overloaded_names": ["L1"], + "prioritized_actions": {}} + res, ctx = _normalize_step1_outcome(context_like) + assert res is None and ctx is context_like + + def test_isinstance_branch_classifies_result_and_context(self, monkeypatch): + # Simulate the real library: AnalysisResult is a class. + class AnalysisResult(dict): + pass + + main_mod = sys.modules["expert_op4grid_recommender.main"] + monkeypatch.setattr(main_mod, "AnalysisResult", AnalysisResult, raising=False) + + early = AnalysisResult({"lines_overloaded_names": ["L1"]}) + assert _normalize_step1_outcome(early) == (early, None) + # A value that is NOT an AnalysisResult is treated as a context. + ctx_like = {"obs": 1} + assert _normalize_step1_outcome(ctx_like) == (None, ctx_like) + + @_needs_real + def test_real_analysis_result_is_short_circuit(self): + early = _RealResult(lines_overloaded_names=["L1"]) + assert _normalize_step1_outcome(early) == (early, None) + + @_needs_real + def test_real_analysis_context_is_proceed(self): + ctx = _RealContext(env="E", lines_overloaded_names=["L1"]) + res, out = _normalize_step1_outcome(ctx) + assert res is None and out is ctx + + +# --------------------------------------------------------------------- +# Service integration: AnalysisMixin.run_analysis_step1 +# --------------------------------------------------------------------- + +def _service_returning(monkeypatch, outcome): + """A RecommenderService stubbed just enough to drive run_analysis_step1, + with the upstream library replaced by a stub returning ``outcome``.""" + service = RecommenderService() + service._dict_action = {} + service._cached_env_context = MagicMock() + monkeypatch.setattr(service, "_ensure_n_state_ready", lambda: None) + monkeypatch.setattr( + service, "_normalize_contingency_elements", + lambda elts: list(elts) if isinstance(elts, (list, tuple)) else [elts], + ) + monkeypatch.setattr(_analysis_mixin, "run_analysis_step1", lambda **kw: outcome) + config.DO_RECO_MAINTENANCE = False + return service + + +class TestServiceStep1Outcome: + def test_legacy_tuple_proceed(self, monkeypatch): + legacy = (None, {"obs": 1, "lines_overloaded_names": ["L1"]}) + res = _service_returning(monkeypatch, legacy).run_analysis_step1(["LINE_A"]) + assert res["can_proceed"] is True + assert res["lines_overloaded"] == ["L1"] + + def test_legacy_tuple_short_circuit(self, monkeypatch): + legacy = ({"lines_overloaded_names": ["L9"]}, None) + service = _service_returning(monkeypatch, legacy) + res = service.run_analysis_step1(["LINE_A"]) + assert res["can_proceed"] is False + assert res["lines_overloaded"] == ["L9"] + assert service._analysis_context is None + + @_needs_real + def test_real_context_union_proceeds(self, monkeypatch): + ctx = _RealContext(obs=object(), lines_overloaded_names=["L1", "L2"]) + service = _service_returning(monkeypatch, ctx) + res = service.run_analysis_step1(["LINE_A"]) + assert res["can_proceed"] is True + assert res["lines_overloaded"] == ["L1", "L2"] + assert service._analysis_context is ctx + + @_needs_real + def test_real_result_union_short_circuits(self, monkeypatch): + result = _RealResult(lines_overloaded_names=["L9"]) + service = _service_returning(monkeypatch, result) + res = service.run_analysis_step1(["LINE_A"]) + assert res["can_proceed"] is False + assert res["lines_overloaded"] == ["L9"] + assert service._analysis_context is None