C-94: the findability preflight — ask the store what the consumer asks

docs/CICs/UNFAOPostProcessorManager.md

@@ -3,7 +3,7 @@
 
 **Status:** Active  
 **Owner:** PRIO MD&D Team  
-**Last reviewed:** 2026-08-17  
+**Last reviewed:** 2026-08-18  
 **Related ADRs:** ADR-001, ADR-002, ADR-008, ADR-009  
 
 ---
@@ -90,10 +90,11 @@ Assumptions that are not met **must cause failure**, not fallback behavior. The
 - **Null values in required metadata columns:** Raises `ValueError` with null count and affected column name (C-01 resolved — validation active)
 - **Dataset initialization failure:** Raises `ValueError` in `_save()` if datasets are None
 - **Appwrite upload failure:** Propagates exception from `DatastoreModule`
+- **Delivery invisible to the consumer:** raises `delivery.findability.DeliveryNotFindableError` (C-94, added 2026-08-18). After both legs are uploaded, the manager queries the partner store as the consumer does — `name == product.CONSUMER_DOCUMENT_NAME`, per category — and refuses a falsy answer. This is the one failure mode where every upload reports success and the consumer still sees nothing; run-0's historical leg stranded exactly that way (C-79). It runs only inside the §11.4 interlock, and queries through a store with pipeline-core's automatic `name == model_name` filter suppressed, so it verifies the declared name rather than the views-models directory name that happens to match (C-77). **It does not detect a delivery that never ran, or stale data served from the consumer's cache** — both recorded as gaps in C-94
 - **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). 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 -->
+- **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.ProducerClientUnavailable`), 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

@@ -275,7 +275,7 @@ Cross-refs: **C-94**, **C-96**, þing-01 `orð_dómr.md` D2, issue #249.
 | ID | C-94 |
 | Tier | 2 — the failure mode is invisible by construction and lands on the live FAO path: upload succeeds, storage is billed, the consumer's endpoint returns empty, nothing raises anywhere. ADR-013 §4.1a's *"invisible to the consumer, not merely degraded."* |
 | Source | `/expert-code-review` of the standing decisions, 2026-08-12 |
-| Trigger | **Re-specified twice on 2026-08-13; the first attempt was not exclusive and its own worked example matched two arms.** (a) A delivery is reported empty **and an upload occurred after the bucket reached the state under investigation** — that is what the preflight below would catch, and the time bound is what the first attempt omitted. (b) `APPWRITE_READ_API_KEY` is provisioned, at which point the deferral has no remaining cost. *(A third arm — "reported empty with no upload since" — was drafted and withdrawn: it is not observable from this repository, which the amendment says four lines on, and ADR-014 §4 requires a trigger someone can notice. It is a gap, and is stated as one below rather than dressed as a trigger.)* |
+| Trigger | **Re-specified twice on 2026-08-13; the first attempt was not exclusive and its own worked example matched two arms.** (a) A delivery is reported empty **and an upload occurred after the bucket reached the state under investigation** — that is what the preflight below would catch, and the time bound is what the first attempt omitted. (b) `APPWRITE_READ_API_KEY` is provisioned, at which point the deferral has no remaining cost. *(A third arm — "reported empty with no upload since" — was drafted and withdrawn: it is not observable from this repository, which the amendment says four lines on, and ADR-014 §4 requires a trigger someone can notice. It is a gap, and is stated as one below rather than dressed as a trigger.)* **Rewritten 2026-08-18, because arm (b) expired without firing** (register conventions: a trigger whose event has already occurred reads identically to a pending one). The preflight was built *without* `APPWRITE_READ_API_KEY`, so "it is provisioned" can no longer arm anything. What remains live is arm (a), now narrowed: **a delivery is reported empty, an upload occurred since, and the preflight did NOT raise** — that combination means the check is looking in the wrong place, and it is the only arm this repository can still be surprised by. |
 | Owner | This repository, for the mechanism. The credential is the operator's. |
 | Location | `views_postprocessing/contract/wire/sink.py` (the upload path, where nothing verifies); `views_postprocessing/delivery/`. |
 
@@ -293,6 +293,21 @@ FAO emailed at **09:15 UTC** that `faoapi.viewsforecasting.org` returned no data
 
 **So the trigger was mis-specified, not the mechanism.** "A delivery is reported empty" names a symptom with at least two causes, and this entry's preflight addresses only one of them.
 
+**Partial mitigation, 2026-08-18 — the preflight is built.** After both legs are uploaded, each manager asks the partner store the question the consumer asks — `name == product.CONSUMER_DOCUMENT_NAME`, `category ∈ {forecast, historical}` — and refuses a falsy answer (`delivery/findability.py`, `DeliveryNotFindableError`). The two legs are checked separately on purpose: a run whose forecast landed and whose historical did not is invisible in exactly one half, and the historical leg is the one that actually stranded in run-0 (C-79).
+
+**Two decisions inside it that a later reader should not have to re-derive:**
+
+1. **It runs on the existing key, not the registry's `APPWRITE_READ_API_KEY` slot.** Verified in the Appwrite console 2026-08-18: the live `VIEWS Pipeline Core` key already carries `documents.read`, `rows.read`, `buckets.read` and `files.read`. A separate read credential would buy no isolation here, because the preflight runs *inside the delivery process*, which already holds the write key it just uploaded with. C-96's permission is about the operation being read-only, and it is. The registry slot stays `planned` for a preflight that runs **outside** the delivery, where the isolation would be real.
+2. **It queries through a store with pipeline-core's automatic `name == model_name` filter suppressed** (`_build_partner_read_store`). `get_latest_file_id` delegates to `get_predictions_by_metadata`, which merges the path manager's model name into every query — so without the suppression the check would verify the views-models *directory* name, which equals the declared consumer name only by coincidence (**C-77**). Verifying the coincidence rather than the contract would leave this green while a rename took the delivery dark, which is the precise failure it exists to see.
+
+**The read-back is scoped to THIS run, and that is the whole guard.** The first implementation asked *"is there any document under the consumer's name for this category"* — a question the **previous** delivery already answers yes to. Documents accumulate across runs (that is what makes "latest" meaningful to the consumer), so from delivery 2 onward the check could never fail: run-2's upload reports success, its metadata document is never created — the exact C-79 shape — the query returns run-1's document, and the preflight logs *"passed"* while the consumer goes on serving run-1. Caught by `/code-review high` before merge. `_ContractStorePort.upload` now returns the uploaded `file_id` (it was discarding it), the sink carries the manifest's id out — it is uploaded last, so it is the newest `category="forecast"` document — and the check asserts the newest document the consumer would find **is the one this run put there**. The refusal distinguishes "nothing found" from "found the previous run's", because those are different operator situations.
+
+**A failed read-back is not an invisible delivery.** `findability.unverified` names that separately (`FindabilityUnverifiedError`): a store error after a successful upload means the delivery is UNVERIFIED, not known invisible, and quarantining on it would be an outage the guard manufactured. Same distinction C-103 draws between a missing producer client and a producer that publishes no boundary, and C-99 between an unrecognised store result and a real one — three instances now of the same rule, that *could not ask* and *asked and got nothing* call for different operator actions.
+
+**A note on which pipeline-core you read, because it changed a review's conclusion.** The same review reported that `unverified()` was dead code, on the grounds that `get_predictions_by_metadata` swallows a failed search and returns `[]` — so a store error would arrive as `None` and be reported as an invisible delivery. That is true of **2.3.0**, which is what the drifted developer venv holds (C-104). It is false of **3.0.1**, which `poetry.lock` pins and CI installs: there the method **raises `MetadataSearchIncomplete`**, with a comment in pipeline-core saying why — *"Returning [] here would tell every caller 'no predictions match', which is a statement about the shelf rather than about the lookup… a false negative to an external counterparty"* (views-pipeline-core C-241, its Cluster J). Verified 2026-08-19 by reading `3.0.1` from the views-pipeline-core checkout rather than the installed package. So the split holds where it runs. **This is C-104's hazard in its most expensive form yet**: not a wall of red, but a confident and wrong conclusion about production drawn from a stale environment.
+
+**The tier does not move, and the reason it does not is the point.** The Tier 2 rationale was *"upload succeeds, storage is billed, the consumer's endpoint returns empty, nothing raises anywhere."* For that cause, something now raises. What holds the entry at 2 is the two causes below, which this does not touch and which remain invisible — the tier now rests on the gaps rather than on the mechanism.
+
 **Two uncovered causes, stated as gaps rather than dressed as triggers.** Neither is observable from here, so neither can be a trigger under ADR-014 §4 — a trigger nobody can notice is a wish:
 
 1. *The bucket is empty because nothing was delivered.* This repository is not told when a delivery is due and has no view of whether the last one is still present. That is the 2026-08-12 case.
@@ -1081,7 +1096,7 @@ So the scenario this entry was filed on — a missing dependency silently shippi
 
 **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.
+**Partial mitigation, 2026-08-17.** `source_metadata` now raises `ProducerClientUnavailable` 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_clone_readiness.py

@@ -49,6 +49,7 @@
     "views_postprocessing.delivery.coverage",
     "views_postprocessing.delivery.draws",
     "views_postprocessing.delivery.parity",
+    "views_postprocessing.delivery.findability",
     "views_postprocessing.delivery.observed_range",
     "views_postprocessing.delivery.provenance",
     "views_postprocessing.contract.wire.sink",