C-103: a missing producer client refuses instead of passing for 'no boundary' — and the Tier 2 premise did not survive verification

docs/CICs/UNFAOPostProcessorManager.md

@@ -3,7 +3,7 @@
 
 **Status:** Active  
 **Owner:** PRIO MD&D Team  
-**Last reviewed:** 2026-08-05  
+**Last reviewed:** 2026-08-17  
 **Related ADRs:** ADR-001, ADR-002, ADR-008, ADR-009  
 
 ---
@@ -93,7 +93,7 @@ Assumptions that are not met **must cause failure**, not fallback behavior. The
 - **Wrong forecast selected:** structurally impossible since #149. Selection is by **run manifest** — a commit marker whose contents are hash-verified — not by scanning the bucket for the newest `category="forecast"` upload. Declared identity is additionally checked **per shard header** against the launched ensemble inside `TargetLease.load()` (`contract/wire/source_selection.py:73-81`), so identity comes from the artifact's own content. The metadata-field check this bullet used to describe (`delivery/identity.py`) was retired in #150 and the legacy reader it served in #149; register C-25 is closed as *superseded by mechanism* <!-- legacy-ok: retirement record -->
 - **Launch config incomplete:** raises `LaunchConfigError` naming the missing key. A launcher that omits `wire_contract` or declares a `data_format` other than `feature_frame` is **refused**, never quietly routed into a fallback (ADR-003, register C-63)
 - **Region coverage mismatch:** Raises `CoverageError` in `_check_coverage()` (called from `_validate()`) if a pinned region's delivered cell count is wrong (S1/C-34) or a GAUL-uncovered excluded cell leaks into the delivery (S4/C-30)
-- **Fabricated historical tail:** `_read_historical_frame()` drops months beyond the producer's `last_valid_month_id` at the read (`_clip_observed_history` was the pandas equivalent, retired with that path in #149) so unobserved zero-padding is not shipped as observed history (S2/C-26); **degrades open** (skips the clip with a WARNING) if the boundary cannot be resolved <!-- legacy-ok: retirement record -->
+- **Fabricated historical tail:** `_read_historical_frame()` drops months beyond the producer's `last_valid_month_id` at the read (`_clip_observed_history` was the pandas equivalent, retired with that path in #149) so unobserved zero-padding is not shipped as observed history (S2/C-26). Two outcomes when the boundary is unavailable, and they are different on purpose (C-103, 2026-08-17): if the producer simply publishes no boundary — or the read fails — it **degrades open**, skipping the clip with a WARNING that states the unobserved tail will ship; if the producer client cannot be imported at all it **refuses** (`source_metadata.ProducerClientMissing`), because a broken environment is not a producer fact <!-- legacy-ok: retirement record -->
 - **Upload provenance:** the historical artifact's `description` carries structured provenance (lookup version, region, expected/actual cell counts, unmapped count) built by `delivery/provenance.py` (`build_provenance` → `compact_description`) via the manager's `_historical_frame_description()` (S5/C-15). The **forecast** side carries no such description: its guarantee is the wire's verified chain — per-shard content hashes recorded in the §4.2 run manifest, header asserts on load, and manifest-last commit ordering. That is identity and integrity, not the C-15 provenance field set; the §4.2 manifest's keys are exactly `contract_version`, `run_id`, `targets`, `shards`, `expected_months`, `expected_cell_count`, `sidecar` — and it carries **no** `lookup_version`, `region` or `unmapped_count`. `_delivery_description()` was the pandas-path equivalent and was deleted with it in #149 <!-- legacy-ok: retirement record -->
 
 The following **must never** fail silently:

reports/technical_risk_register.md

@@ -1064,15 +1064,24 @@ Recorded rather than left implicit because "the operator did a console session"
 | Field | Value |
 |-------|-------|
 | ID | C-103 |
-| Tier | 2 — structural fragility with a named scenario: the guard's dependency is absent, the absence is caught by a bare `except Exception`, and the delivery proceeds unclipped. Held at 2 rather than 1 because of a gap, not a judgement — see the verification question below. |
+| Tier | **3** — re-tiered down from 2 on 2026-08-17, when the verification question below was answered and the premise did not hold. The residual is real but narrower: not "the dependency is missing", which a different guard already refuses, but "any failure to read the boundary degrades open". |
 | Source | `/repo-assimilation` (2026-08-16), measured |
-| Trigger | When the launcher's environment is next built or changed — a views-models deploy, a container rebuild, a `poetry install` on the delivery host — verify `import datafactory_query` succeeds there. The delivery will not tell you if it does not. |
+| Trigger | When a delivery logs *"last_valid_month_id could not be read; skipping the observed-range clip"* — that is now the only route to an unclipped delivery, and it is a real one (a network failure or a reshaped `.zattrs` reaches it). Decide then whether degrade-open is still the right side for that case, or whether the partner should be told the tail is unverified. |
 | Owner | This repository, for the swallow and the declaration. The producer owns the fact itself. |
-| Location | `views_postprocessing/contract/source_metadata.py:37`; `views_postprocessing/unfao/managers/unfao.py:137-142`, `views_postprocessing/crafd/managers/crafd.py:137-142`; `pyproject.toml` (the dependency is absent) |
+| Location | `views_postprocessing/contract/source_metadata.py:74` (the classification); `views_postprocessing/unfao/managers/unfao.py:137-151`, `views_postprocessing/crafd/managers/crafd.py:137-151` (the two branches). **Not `pyproject.toml`** — the original entry listed it as a risk site on the assumption the dependency was undeclared everywhere; it is the launcher's to declare and both launchers do, so there is nothing to add here. Line numbers re-read 2026-08-17 after the fix moved them. |
 
 `source_metadata.last_valid_month_id` lazily imports `datafactory_query.defaults`. Measured 2026-08-16: that package is in neither `pyproject.toml` nor `poetry.lock`, and `import datafactory_query` raises `ModuleNotFoundError` in the project venv. Its only caller wraps the call in `except Exception: lv = None` and then returns the historical frame **unclipped**, logging one WARNING — so "the dependency is missing" and "the producer publishes no boundary attribute" leave through the same branch with the same outcome, and that outcome is unobserved zero-padded months shipping to the partner as observed history. The lazy import states its own reason — *"so this module loads without the heavy datafactory dependency present (e.g. in unit-test environments)"* — but nothing at the call site distinguishes a unit-test environment from a delivery.
 
-**Verification question, stated as a gap rather than dressed as a finding.** Whether the production launcher's environment supplies `datafactory_query` could not be established from this repository; views-models builds that environment. If it does, this is a declaration gap and the swallow is the whole risk. If it does not, C-26's fabrication has been shipping unclipped since #126. **Do not downgrade on inspection of this repo alone** — the instruction C-26 already carries, for the same reason.
+**ANSWERED 2026-08-17, and the answer moves the tier.** The question was whether the production launcher supplies `datafactory_query`. It does, twice over:
+
+- views-models `postprocessors/un_fao/requirements.txt` and `postprocessors/un_crafd/requirements.txt` both pin **`views-datafactory>=1.9.0,<2.0.0`**, which is what ships the `datafactory_query` module (there is no separate distribution — `pip download datafactory-query` finds nothing; it is one of nine packages in views-datafactory's wheel).
+- More decisively, the postprocessor's `config_queryset.py` imports `datafactory_query.defaults` **at module scope** and raises a `RuntimeError` naming the fix if it is absent. A missing client therefore makes `get_queryset()` return `None`, which `launch_config.assert_queryset_was_importable` turns into a refusal (**C-83**) *before* `_read_historical_frame` is ever reached.
+
+So the scenario this entry was filed on — a missing dependency silently shipping fabricated months on the live path — **cannot occur**. It was already guarded, by a check written for a different reason. The Tier 2 rested on a premise measured only in *this* repository's venv, and the instruction it borrowed from C-26 (*"do not downgrade on inspection of this repo alone"*) was the right instruction: the resolution came from reading the launcher, not from reading here.
+
+**What is genuinely left, and it is why this stays open at Tier 3.** The `except Exception` was never only about the import. A network failure, an auth error, a reshaped `.zattrs`, a timeout — all still leave through one branch, log one WARNING, and deliver the unobserved tail as observed history. That is the recorded C-26 degrade-open decision applied far more broadly than C-26 argued for.
+
+**Partial mitigation, 2026-08-17.** `source_metadata` now raises `ProducerClientMissing` instead of letting the import failure fall into the caller's broad `except`, and both managers re-raise it rather than degrading. Defence in depth for the paths `assert_queryset_was_importable` does not cover — a direct caller, a future launcher, a partner not going through the same queryset. The module also gained its first tests (`tests/test_source_metadata.py`, 7, mutation-proven against both the pre-fix return-`None` and a manager that collapses the two branches back into one); it had **none** before, which is how "return None like everything else" ever looked reasonable. The broad degrade-open is deliberately unchanged — narrowing it is a decision about what to tell the partner, not a refactor.
 
 **C-60 is this shape, and it was resolved by deleting the degradation.** There, a provenance stamp reached into the producer's ledger schema inside a bare `except … pass` and returned `"unknown"`; the fix was to raise. The difference here is that the degradation is deliberate and documented ("degrade-open, C-26") — which makes the question *whether the open side is still the right one*, not whether someone forgot.
 

tests/test_source_metadata.py

@@ -0,0 +1,197 @@
+"""The producer-fact seam: what happens when the producer cannot be asked (C-103).
+
+``contract/source_metadata.py`` is the single place this repository asks
+views-datafactory for a data fact, and until 2026-08-17 it had **no tests at all** —
+which is how the defect below survived: nothing described what the module should do
+when the client is absent, so "return None like everything else" looked reasonable.
+
+The distinction these tests pin is the whole point of the module:
+
+* the producer publishes **no boundary** — a normal, older store. The caller degrades
+  open and delivers unclipped (C-26, a recorded decision).
+* the producer **cannot be asked at all** — a broken environment. Degrading open here
+  ships the unobserved zero-padded tail as observed history, and does it with one
+  WARNING in a log nobody reads.
+
+Same shape as C-60, where a provenance stamp degraded to ``"unknown"`` on a bare
+except and made every delivery untraceable in exactly the field it existed to answer.
+"""
+
+from __future__ import annotations
+
+import sys
+import types
+from pathlib import Path
+
+import pytest
+
+from tests.conftest import PARTNER_PACKAGES
+from views_postprocessing.contract import source_metadata
+
+_REPO = Path(__file__).resolve().parent.parent
+
+#: The numpy 1.x/2.x ABI break recorded in views-models
+#: `postprocessors/un_fao/requirements.txt` on 2026-08-13, found by the pre-delivery
+#: rehearsal. It is a `ValueError`, not an `ImportError` — which is why the guard
+#: catches `Exception`.
+_ABI_BREAK = (
+    "numpy.dtype size changed, may indicate binary incompatibility. "
+    "Expected 96 from C header, got 88"
+)
+
+
+@pytest.fixture
+def absent_client(monkeypatch):
+    """Make `datafactory_query` unimportable REGARDLESS of what is installed.
+
+    The first draft of this module relied on the package happening to be absent in the
+    developer venv, which meant four of these tests would have failed in the launcher
+    prefix — the environment they describe. That is the C-30/C-46 shape again: a guard
+    whose proof rests on an ambient property nothing declares.
+    """
+    monkeypatch.setitem(sys.modules, "datafactory_query", None)
+    monkeypatch.setitem(sys.modules, "datafactory_query.defaults", None)
+
+
+@pytest.fixture
+def exploding_client(monkeypatch):
+    """`datafactory_query` is INSTALLED and raises on load — the real-world case."""
+
+    class _Exploding:
+        __name__ = "datafactory_query.defaults"
+
+        def __getattr__(self, name):
+            raise ValueError(_ABI_BREAK)
+
+    module = types.ModuleType("datafactory_query")
+    defaults = _Exploding()
+    module.defaults = defaults
+    monkeypatch.setitem(sys.modules, "datafactory_query", module)
+    monkeypatch.setitem(sys.modules, "datafactory_query.defaults", defaults)
+
+
+def test_an_absent_client_raises_rather_than_reporting_no_boundary(absent_client):
+    with pytest.raises(source_metadata.ProducerClientUnavailable):
+        source_metadata.last_valid_month_id()
+
+
+def test_an_absent_client_is_told_to_install_the_package(absent_client):
+    with pytest.raises(source_metadata.ProducerClientUnavailable) as excinfo:
+        source_metadata.last_valid_month_id()
+    message = str(excinfo.value)
+    assert "not installed" in message
+    assert "views-datafactory" in message, "the refusal must name the package to install"
+    # The reader must be able to tell this apart from the degrade-open case.
+    assert "observed" in message, (
+        "the refusal must say what degrading open would have cost — unobserved months "
+        "shipping as observed history — or it reads as a routine unavailability"
+    )
+
+
+def test_a_client_that_raises_on_load_is_NOT_reported_as_missing(exploding_client):
+    """The failure this environment actually had: a ValueError, not an ImportError.
+
+    An `except ImportError` clause would let this reach the caller's degrade-open and
+    ship the unobserved tail — the precise case the guard exists to separate.
+    """
+    with pytest.raises(source_metadata.ProducerClientUnavailable) as excinfo:
+        source_metadata.last_valid_month_id()
+    message = str(excinfo.value)
+    assert "present but raised while loading" in message
+    assert "ValueError" in message and "numpy.dtype size changed" in message, (
+        "the refusal must quote what actually went wrong; the operator cannot act on "
+        "'could not be loaded' alone"
+    )
+    assert "Do NOT reinstall" in message, (
+        "reporting an installed-but-broken package as missing sends the operator to a "
+        "fix they have already applied"
+    )
+
+
+def test_the_refusal_chains_the_original_exception(absent_client):
+    """``raise ... from exc``: the traceback must still say what actually failed."""
+    with pytest.raises(source_metadata.ProducerClientUnavailable) as excinfo:
+        source_metadata.last_valid_month_id()
+    assert isinstance(excinfo.value.__cause__, ImportError)
+
+
+def test_it_is_logged_as_well_as_raised(caplog, absent_client):
+    """ADR-008: a refusal on the delivery path is logged persistently AND raised."""
+    with caplog.at_level("ERROR"), pytest.raises(source_metadata.ProducerClientUnavailable):
+        source_metadata.last_valid_month_id()
+    assert any(r.levelname == "ERROR" for r in caplog.records), (
+        "the refusal was raised but never logged; a traceback that dies inside a "
+        "scheduled run leaves no persistent record (ADR-008)"
+    )
+
+
+def test_the_producers_answer_passes_through_untouched(monkeypatch):
+    """When the client IS present, this module reads and returns — it does not decide.
+
+    Including ``None``: a store that predates the attribute reports no boundary, and
+    that is the case the caller is entitled to degrade open on.
+    """
+    import types
+
+    for answer in (137, None):
+        fake = types.ModuleType("datafactory_query")
+        defaults = types.ModuleType("datafactory_query.defaults")
+        seen = {}
+
+        def get_last_valid_month_id(zarr_url=None, _answer=answer):
+            seen["zarr_url"] = zarr_url
+            return _answer
+
+        defaults.get_last_valid_month_id = get_last_valid_month_id
+        fake.defaults = defaults
+        monkeypatch.setitem(sys.modules, "datafactory_query", fake)
+        monkeypatch.setitem(sys.modules, "datafactory_query.defaults", defaults)
+
+        assert source_metadata.last_valid_month_id("zarr://declared") == answer
+        assert seen["zarr_url"] == "zarr://declared", (
+            "the declared store must be passed through; guessing the producer's "
+            "default here would be the inference ADR-003 forbids"
+        )
+
+
+@pytest.mark.parametrize("partner", PARTNER_PACKAGES)
+def test_the_managers_do_not_swallow_the_refusal(partner):
+    """The degrade-open must not re-absorb what this module just refused.
+
+    Parametrized over ``PARTNER_PACKAGES`` rather than a literal pair: eight guards
+    once hardcoded ``"unfao"`` and all eight went on passing over ``crafd/`` when it
+    landed (see ``tests/conftest.py``). A ninth would have been this one.
+
+    A source check rather than a behavioural one, deliberately: constructing a manager
+    needs pipeline-core, a path manager and an Appwrite environment (C-40), and the
+    property worth pinning is one line — that ``ProducerClientUnavailable`` is re-raised
+    *before* the broad ``except``. Someone tidying the two branches back into one is
+    precisely how C-103 would return, and it would return silently.
+    """
+    source = (_REPO / "views_postprocessing" / partner / "managers" / f"{partner}.py").read_text()
+
+    # Scope to `_read_historical_frame`. Comparing offsets across the whole file would
+    # let a narrow branch on some unrelated `try` satisfy the ordering while the
+    # boundary read's branch was gone.
+    start = source.index("def _read_historical_frame")
+    body = source[start:source.index("\n    def ", start)]
+
+    narrow = body.count("except source_metadata.ProducerClientUnavailable:")
+    broad = body.count("except Exception:")
+    assert narrow == 1, (
+        f"{partner}'s _read_historical_frame has {narrow} ProducerClientUnavailable "
+        "branches, expected 1. Without it a broken environment is indistinguishable "
+        "from a producer that publishes no boundary, and the delivery ships fabricated "
+        "months either way (C-103)."
+    )
+    assert broad == 1, (
+        f"{partner}'s _read_historical_frame has {broad} broad `except Exception:` "
+        "branches, expected 1 (the C-26 degrade-open). If it is gone the refusal may "
+        "be fine, but this check no longer describes the code — read it and rewrite it."
+    )
+    assert body.index("except source_metadata.ProducerClientUnavailable:") < body.index(
+        "except Exception:"
+    ), (
+        f"{partner} catches Exception before ProducerClientUnavailable, so the narrow "
+        "branch is unreachable"
+    )