A1 — ADR-003 E2 load-time stamp guard (D-18, closes #86)

reports/technical_risk_register.md

@@ -3,7 +3,7 @@
 **Last updated:** 2026-06-28
 **Governing ADR:** Pending (will be added when base docs are adopted)
 **Total entries:** 36
-**Concerns:** Open 32 | Resolved 2 | Invalidated 2
+**Concerns:** Open 31 | Resolved 3 | Invalidated 2
 
 > **ID convention:** This register uses the `D-xx` (Debt) prefix for all concern entries; there are no disagreement entries. IDs are permanent and sequential.
 
@@ -141,19 +141,6 @@
 
 ---
 
-### D-18 — Raw-I/O contract is unfalsifiable in CI until guards E1/E2 and the gate key are implemented
-
-| Field | Value |
-|---|---|
-| **Tier** | 2 |
-| **Trigger** | When a contributor reintroduces or adds an internal target transform to `StepshifterModel` before the ADR-003 characterization test (E1), load-time guard (E2), and the gate `target_transform` key are implemented. |
-| **Source** | expert-review (2026-06-08) |
-| **Status** | Partially resolved (2026-06-13) — gate key + E1-equivalent test shipped; **E2 load-time guard still Open** |
-| **Location** | ADR-003 §"Enforcement required at ratification"; ~~`reproducibility_gate.py:24` (`CORE_GENOME` lacks `target_transform`)~~ → now `:25` (key present, validated steps 5–7); E1-equivalent test now in `tests/test_target_transforms.py`; **E2 still absent** (no load-time guard) |
-| **Notes** | ADR-003 ratifies a raw-in/raw-out contract but is documentation-only until guards E1 (fit→predict characterization test asserting raw-space output) and E2 (load-time transform-name guard) ship and `target_transform` is added to the gate. Until then, a future silent-transform commit — a `5fcfe43` rhyme — passes CI green, which is exactly the regression that produced D-17. The fix is cheap (E1 needs no registry) and should not wait for the full declarative `TRANSFORMS` mechanism. See also D-09 (gate omits keys models dereference), D-10 (gate runs only on the train path), and D-13 (existing tests verify wiring, not numerical behavior). **Update (2026-06-13, repo-assimilation):** this entry's premise is now **substantially stale**. `target_transform` IS in `CORE_GENOME` (`reproducibility_gate.py:25`) and is validated against the closed registry (steps 5–7), and an **E1-equivalent characterization test** ships and is green (`tests/test_target_transforms.py::test_predict_routes_through_inverse_exactly_once_end_to_end`, plus the `_process_data`-applies-forward-once and identity-noop tests). A silent-transform commit no longer passes CI unflagged. **What remains Open:** the ADR-003 **E2 load-time guard** (fail-loud on a missing `_target_transform_name` stamp when unpickling a pre-contract artifact) is still unimplemented — this is the residual gap and the one still tied to D-17. The doc drift this staleness reflects (ADR-001/CIC under-describe the shipped contract) is tracked separately in **D-31**. |
-
----
-
 ### D-19 — ADR-003 depends on a not-yet-ratified views-pipeline-core platform ADR (dangling `⟨PENDING⟩` reference)
 
 | Field | Value |
@@ -467,6 +454,18 @@
 
 ---
 
+### D-18 — Raw-I/O contract is unfalsifiable in CI until guards E1/E2 and the gate key are implemented — RESOLVED
+
+| Field | Value |
+|---|---|
+| **Tier** | 2 |
+| **Original trigger** | When a contributor reintroduces or adds an internal target transform to `StepshifterModel` before the ADR-003 characterization test (E1), load-time guard (E2), and the gate `target_transform` key are implemented. |
+| **Source** | expert-review (2026-06-08) |
+| **Status** | Resolved (2026-06-21) — all three ADR-003 enforcement obligations now ship |
+| **Resolution** | ADR-003 mandated three enforcement artifacts, all now present: (1) the gate `target_transform` key is in `CORE_GENOME` (`reproducibility_gate.py:25`), validated against the closed registry (steps 5–7); (2) the **E1** characterization test ships and is green (`tests/test_target_transforms.py::test_predict_routes_through_inverse_exactly_once_end_to_end`, plus `_process_data`-applies-forward-once and identity-noop tests); (3) the **E2** load-time guard now ships (Story A1, #86) — `StepshifterManager._guard_loaded_artifact` raises `PreContractArtifactError` after each `pickle.load` (`stepshifter_manager.py:29-49`, called at the evaluate/forecast load sites) when a loaded artifact lacks the `_target_transform_name` stamp, closing the frozen-pre-contract-artifact ambiguity that tied this to D-17. The guard is presence-only (a legit `log1p` artifact loads fine), per ADR-003 E2. The ADR-001/CIC doc-drift this entry once noted is tracked separately in **D-31** (Workstream C). |
+
+---
+
 ### D-41 — Stepshifter feeds the PRIO-GRID cell id to the regressor as a feature (darts static-covariate default) → outlier cells smeared into latitude-wide "stripes" — RESOLVED
 
 | Field | Value |

tests/test_stepshifter_manager.py

@@ -4,6 +4,7 @@
 from pathlib import Path
 from unittest.mock import MagicMock, patch, mock_open
 from views_stepshifter.manager.stepshifter_manager import StepshifterManager
+from views_stepshifter.infrastructure.exceptions import PreContractArtifactError
 from views_pipeline_core.managers.model import ModelPathManager
 from views_pipeline_core.cli.args import ForecastingModelArgs
 
@@ -354,6 +355,57 @@ def test_forecast_model_artifact(stepshifter_manager):
         assert path_artifact == Path("/path/to/artifacts/non_default_artifact.pkl")
         mock_logger.exception.assert_called_once_with(f"Model artifact not found at {path_artifact}")
 
+def test_load_guard_rejects_pre_contract_artifact_on_evaluate(stepshifter_manager):
+    """ADR-003 E2: an artifact missing `_target_transform_name` (a pre-contract
+    artifact) has an unknown prediction scale and MUST fail loud on load — never be
+    silently assumed to be raw space."""
+    # spec without `_target_transform_name` => hasattr(...) is False, unlike a bare MagicMock
+    pre_contract_model = MagicMock(spec=["predict"])
+    with patch("builtins.open", mock_open(read_data=b"mocked_data")), \
+        patch("pickle.load", MagicMock(return_value=pre_contract_model)):
+
+        args = ForecastingModelArgs(run_type="test_run_type", evaluate=True, saved=True)
+        stepshifter_manager.configs = vars(args)
+
+        with pytest.raises(PreContractArtifactError):
+            stepshifter_manager._evaluate_model_artifact("test_eval_type", None)
+        # the guard fires before prediction is attempted
+        pre_contract_model.predict.assert_not_called()
+
+
+def test_load_guard_rejects_pre_contract_artifact_on_forecast(stepshifter_manager):
+    """Same E2 guard on the forecast path (`_forecast_model_artifact`)."""
+    pre_contract_model = MagicMock(spec=["predict"])
+    with patch("builtins.open", mock_open(read_data=b"mocked_data")), \
+        patch("pickle.load", MagicMock(return_value=pre_contract_model)):
+
+        args = ForecastingModelArgs(run_type="forecasting", forecast=True, saved=True)
+        stepshifter_manager.configs = vars(args)
+
+        with pytest.raises(PreContractArtifactError):
+            stepshifter_manager._forecast_model_artifact(None)
+        pre_contract_model.predict.assert_not_called()
+
+
+def test_load_guard_accepts_stamped_artifact(stepshifter_manager):
+    """The guard checks for the *presence* of the stamp, not its value: a legitimate
+    artifact carrying a non-identity stamp (e.g. `log1p`) loads and predicts normally."""
+    stamped_model = MagicMock(spec=["predict", "_target_transform_name"])
+    stamped_model._target_transform_name = "log1p"
+    stamped_model.predict.return_value = ["mock_df"]
+    with patch("builtins.open", mock_open(read_data=b"mocked_data")), \
+        patch("pickle.load", MagicMock(return_value=stamped_model)), \
+        patch.object(StepshifterManager, "_get_standardized_df", return_value="standardized_df"):
+
+        args = ForecastingModelArgs(run_type="forecasting", forecast=True, saved=True)
+        stepshifter_manager.configs = vars(args)
+
+        result = stepshifter_manager._forecast_model_artifact(None)
+
+        stamped_model.predict.assert_called_once_with("forecasting")
+        assert result == "standardized_df"
+
+
 def test_evaluate_sweep(stepshifter_manager):
     """
     Test the _evaluate_sweep method.

views_stepshifter/infrastructure/exceptions.py

@@ -8,3 +8,15 @@ class MissingHyperparameterError(ReproducibilityError):
     """Raised when a mandatory hyperparameter is missing from the config."""
 
     pass
+
+
+class PreContractArtifactError(ReproducibilityError):
+    """Raised when a loaded model artifact predates the target-transform contract.
+
+    Per ADR-003 E2 (raw-target-space I/O contract): an artifact missing the
+    ``_target_transform_name`` stamp has an unknown prediction scale that cannot
+    be safely inverted, so it must be rejected loudly at load time rather than
+    silently assumed to be raw space.
+    """
+
+    pass

views_stepshifter/manager/stepshifter_manager.py

@@ -4,6 +4,7 @@
 from views_stepshifter.models.hurdle_model import HurdleModel
 from views_stepshifter.models.shurf_model import ShurfModel
 from views_stepshifter.infrastructure.reproducibility_gate import ReproducibilityGate
+from views_stepshifter.infrastructure.exceptions import PreContractArtifactError
 import logging
 import pickle
 import pandas as pd
@@ -24,6 +25,30 @@ def __init__(
         self._is_hurdle = self._config_meta["algorithm"] == "HurdleModel"
         self._is_shurf = self._config_meta["algorithm"] == "ShurfModel"
 
+    @staticmethod
+    def _guard_loaded_artifact(model: Any, path_artifact) -> None:
+        """ADR-003 E2 load-time guard.
+
+        A loaded artifact must declare the target-transform space it predicts in.
+        A pre-contract artifact (missing ``_target_transform_name``) has an unknown
+        prediction scale that cannot be safely inverted, so it is rejected loudly
+        rather than silently assumed to be raw space.
+
+        Note: the guard checks for the *presence* of the stamp, not its value — a
+        legitimate plain-model artifact may carry a non-identity transform (e.g.
+        ``log1p``) and must load fine.
+        """
+        if not hasattr(model, "_target_transform_name"):
+            msg = (
+                f"Model artifact at {path_artifact} predates the target-transform "
+                f"contract (ADR-003 E2): it carries no `_target_transform_name` stamp, "
+                f"so its prediction space is unknown and cannot be safely inverted. "
+                f"Re-train and re-save the artifact under the current contract before "
+                f"evaluating or forecasting."
+            )
+            logger.error(msg)
+            raise PreContractArtifactError(msg)
+
     @staticmethod
     def _get_standardized_df(df: pd.DataFrame) -> pd.DataFrame:
         """
@@ -166,6 +191,8 @@ def _evaluate_model_artifact(
             logger.exception(f"Model artifact not found at {path_artifact}")
             raise
 
+        StepshifterManager._guard_loaded_artifact(stepshift_model, path_artifact)
+
         df_predictions = stepshift_model.predict(run_type, eval_type)
         df_predictions = [
             StepshifterManager._get_standardized_df(df) for df in df_predictions
@@ -209,6 +236,8 @@ def _forecast_model_artifact(self, artifact_name: str) -> pd.DataFrame:
             logger.exception(f"Model artifact not found at {path_artifact}")
             raise
 
+        StepshifterManager._guard_loaded_artifact(stepshift_model, path_artifact)
+
         df_prediction = stepshift_model.predict(run_type)
         df_prediction = StepshifterManager._get_standardized_df(df_prediction)