diff --git a/.github/workflows/run_pytest.yml b/.github/workflows/run_pytest.yml index 6f6cea6..b5dc420 100644 --- a/.github/workflows/run_pytest.yml +++ b/.github/workflows/run_pytest.yml @@ -19,6 +19,46 @@ jobs: - name: Checkout repository uses: actions/checkout@v3 + # ── sibling checkouts: what CI can verify that a bare checkout cannot ──────── + # + # `tests/conftest.sibling_repo` resolves $VIEWS_, else the conventional + # `../` directory. In a one-repo CI checkout neither exists, so every + # cross-repo assertion skips — silently and by design, because a missing sibling + # is normal. The consequence was that CI verified strictly LESS than a developer's + # laptop, and precisely on the guards that matter most across a repo boundary. + # + # Measured 2026-08-03 (402 collected in all three runs, so the delta is entirely + # skips): + # local, all siblings present 362 passed / 40 xfailed / 0 skipped + # CI, bare checkout 347 passed / 38 xfailed / 17 skipped + # CI, with the checkout below 348 passed / 38 xfailed / 16 skipped + # + # So this recovers exactly ONE test — the cross-seam consumer-name check for CRAF'd. + # That is worth having (it is the §4.1a pin, whose failure mode is a delivery nobody + # can find) but it is one, not seventeen. The other sixteen are below. + # + # Only views-crafdapi is checked out, and the choice is deliberate: + # + # views-crafdapi PUBLIC, and its check reads source text -> enabled here. + # views-datafactory PUBLIC, but its 8 tests need the producer's raw GAUL + # parquets (`data/raw/gaul_admin/*.parquet`), which are NOT in + # its git repository. Checking it out converts an honest skip + # into a FileNotFoundError. Measured, not assumed — it was tried. + # views-appwrite PRIVATE. The registry-drift detectors live here and are the + # views-faoapi most valuable of the lot; enabling them needs a token, which + # is an operator decision. Register C-46 and C-57 both carry + # that as their standing residual, and it is still open. + # `actions/checkout` refuses a path outside $GITHUB_WORKSPACE, so the siblings go + # inside it and are pointed at by the declared env vars instead of the conventional + # `../` location. `sibling_repo` checks $VIEWS_ first, precisely so the + # location can be declared rather than assumed. + - name: Checkout views-crafdapi (sibling) + uses: actions/checkout@v3 + with: + repository: views-platform/views-crafdapi + path: _siblings/views-crafdapi + fetch-depth: 0 + - name: Set up Python uses: actions/setup-python@v4 with: @@ -33,6 +73,8 @@ jobs: poetry install - name: Run tests + env: + VIEWS_CRAFDAPI: ${{ github.workspace }}/_siblings/views-crafdapi run: | set -e poetry run pytest tests/ diff --git a/README.md b/README.md index f2e071f..29460ae 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ The **post-forecast delivery layer** for the **VIEWS** (Violence Early-Warning S pipeline. It takes finished VIEWS forecasts, enriches them with geographic metadata, guards their integrity, and delivers them to a partner store. -The only live delivery today is the **UN FAO** path — its product in `views_postprocessing/unfao/`, running on the partner-neutral machinery in `contract/`. +Two partner deliveries run on the same partner-neutral machinery in `contract/`: the **UN FAO** path (`views_postprocessing/unfao/`), serving FAO-FSFC since 2026-07-27, and **CRAF'd** (`views_postprocessing/crafd/`), added 2026-08-03 with its upload interlock still closed. > **New here? Read [`docs/architecture/role_and_seams.md`](docs/architecture/role_and_seams.md) first.** > It explains what this repo is, how it relates to pipeline-core / faoapi / datafactory, @@ -50,7 +50,7 @@ Requires **Python 3.11–3.14**. | Package | Version | Why | |---------|---------|-----| | `views-pipeline-core` | `>=2.1.3,<3.0.0` | The framework: lifecycle base classes, data loader, dataset container, Appwrite/datastore tools | -| `views-frames` | `>=1.0,<2` | The frame data contract — **the live delivery representation** since #126. pandas survives only in `contract/enrichment.py` (the build/verification path) | +| `views-frames` | `>=1.10.2,<2` | The frame data contract — **the live delivery representation** since #126. pandas survives only in `contract/enrichment.py` (the build/verification path) | --- @@ -75,7 +75,7 @@ In practice the manager is constructed and run by **views-models** | **Read** | `_read_historical_frame`, `_read_forecast_data_contract` | Historical actuals from views-datafactory arrive **frame-native** (#126); the forecast run is resolved from the Appwrite store by its **run manifest**, with each shard's header verified on load (ADR-013 §4.3). | | **Transform** | `_transform` | Resolution only. Prediction values are **not** transformed — no collapse, no reconciliation. | | **Validate** | `_validate`, `_check_coverage` | Asserts the read resolved, then enforces the region coverage + GAUL-excluded-cell contract (C-34 / C-30). The metadata null-gate fires later, at artifact build (`contract/historical.assert_metadata_complete`). | -| **Save** | `_save` → `_save_contract` | Builds the ADR-013 wire — arrow shards, the §5 GAUL sidecar, the historical artifact — commits the run **manifest last**, and stamps each upload with structured provenance (C-15). | +| **Save** | `_save` → `_save_contract` | Builds the ADR-013 wire — arrow shards, the §5 GAUL sidecar, the historical artifact — commits the run **manifest last**. The historical artifact carries structured provenance in its store-document `description` (C-15); the forecast leg's uploads carry `{name, category, loa, filename, doc_type, targets}` and **no `description`** — a gap, not a design. | The pandas metadata-join and history-clip stages were retired with the legacy delivery path in #149; their rules survive as called invariants under `delivery/`. See the @@ -89,19 +89,28 @@ retraction primitive; a correction is a new complete run, manifest last. ### Output schema (geographic metadata columns) -These 9 columns are the delivered geography contract (declared in `contract/gaul_schema.py`): - -| Column | Type | Description | -|--------|------|-------------| -| `pg_xcoord` | float | PRIO-GRID cell centroid longitude | -| `pg_ycoord` | float | PRIO-GRID cell centroid latitude | -| `country_iso_a3` | str | ISO 3166-1 alpha-3 country code | -| `admin1_gaul0_code` | int | GAUL level-0 (country) code | -| `admin1_gaul0_name` | str | GAUL level-0 (country) name | -| `admin1_gaul1_code` | int | GAUL level-1 (province) code | -| `admin1_gaul1_name` | str | GAUL level-1 (province) name | -| `admin2_gaul2_code` | int | GAUL level-2 (district) code | -| `admin2_gaul2_name` | str | GAUL level-2 (district) name | +These 9 columns are the delivered geography contract, declared in +`contract/gaul_schema.py`. **The order below is normative** (ADR-013 §5.1) and is +byte-pinned by the §10 golden fixture — a reader that reorders them reads the wrong +column. `tests/test_doc_accuracy.py` checks this table against the declaration. + +| Column | Wire type | Description | +|--------|-----------|-------------| +| `pg_xcoord` | float64 | PRIO-GRID cell centroid longitude | +| `pg_ycoord` | float64 | PRIO-GRID cell centroid latitude | +| `country_iso_a3` | string | ISO 3166-1 alpha-3 country code | +| `admin1_gaul1_code` | float64 | GAUL level-1 (province) code | +| `admin1_gaul1_name` | string | GAUL level-1 (province) name | +| `admin1_gaul0_code` | float64 | GAUL level-0 (country) code | +| `admin1_gaul0_name` | string | GAUL level-0 (country) name | +| `admin2_gaul2_code` | float64 | GAUL level-2 (district) code | +| `admin2_gaul2_name` | string | GAUL level-2 (district) name | + +*(Corrected 2026-08-03: this table had `admin1_gaul0_*` before `admin1_gaul1_*` — +the reverse of the normative order — and typed the four `*_code` columns `int`. They +are **float64 on the wire, always**, by the §5.1 ruling: the codes are nullable and +arrow has no nullable int in this contract. Both errors survived because nothing +compared the table to the declaration.)* --- @@ -158,23 +167,27 @@ See [`docs/CLONING.md`](docs/CLONING.md). ## Configuration -The FAO delivery reads Appwrite connection settings from the environment. The required -names are **declared** in `unfao/appwrite_env.py` and validated fail-loud before any store -is constructed — a missing or empty variable raises, naming every one that is absent, -rather than half-configuring a client. Coordinates come from the Appwrite Seam Contract registry -(referenced by URL, never copied); the API key is an operator slot: +Each delivery reads Appwrite connection settings from the environment. The required +names are **declared** per partner — `unfao/appwrite_env.py`, `crafd/appwrite_env.py` — +and validated fail-loud before any store is constructed: a missing or empty variable +raises, naming every one that is absent, rather than half-configuring a client. + +**The names are below; the values are not.** Coordinates live in the Appwrite Seam +Contract's registry, which this repo references by pinned URL and never copies (þing-01 +sáttmál S6 — copies were the platform's original failure). The launcher supplies the +values; the API key is an operator slot. ```bash -# Appwrite connection (secrets) -APPWRITE_ENDPOINT=https://cloud.appwrite.io/v1 +# Appwrite connection +APPWRITE_ENDPOINT=... APPWRITE_DATASTORE_PROJECT_ID=... -APPWRITE_DATASTORE_API_KEY=... +APPWRITE_DATASTORE_API_KEY=... # operator-issued secret -# Production-forecasts store (input) -APPWRITE_PROD_FORECASTS_BUCKET_ID=production_forecasts -APPWRITE_PROD_FORECASTS_BUCKET_NAME=Production Forecasts -APPWRITE_PROD_FORECASTS_COLLECTION_ID=production_forecasts -APPWRITE_PROD_FORECASTS_COLLECTION_NAME=Production Forecasts +# Production-forecasts store (input — shared by every partner) +APPWRITE_PROD_FORECASTS_BUCKET_ID=... +APPWRITE_PROD_FORECASTS_BUCKET_NAME=... +APPWRITE_PROD_FORECASTS_COLLECTION_ID=... +APPWRITE_PROD_FORECASTS_COLLECTION_NAME=... # UN FAO store (output) APPWRITE_UNFAO_BUCKET_ID=... @@ -182,11 +195,21 @@ APPWRITE_UNFAO_BUCKET_NAME=... APPWRITE_UNFAO_COLLECTION_ID=... APPWRITE_UNFAO_COLLECTION_NAME=... -# Metadata database +# CRAF'd store (output) +APPWRITE_CRAFD_BUCKET_ID=... +APPWRITE_CRAFD_BUCKET_NAME=... +APPWRITE_CRAFD_COLLECTION_ID=... +APPWRITE_CRAFD_COLLECTION_NAME=... + +# Metadata database (shared) APPWRITE_METADATA_DATABASE_ID=... APPWRITE_METADATA_DATABASE_NAME=... ``` +*(Corrected 2026-08-03: four production-forecasts coordinate **values** were written out +above, two lines below the sentence saying they never are. The value-copy guard scanned +only `.py`; it now scans markdown too.)* + --- ## Documentation diff --git a/docs/ADRs/014_claims_and_the_guards_that_carry_them.md b/docs/ADRs/014_claims_and_the_guards_that_carry_them.md index 13292df..547c9f3 100644 --- a/docs/ADRs/014_claims_and_the_guards_that_carry_them.md +++ b/docs/ADRs/014_claims_and_the_guards_that_carry_them.md @@ -13,7 +13,8 @@ infrastructure), [ADR-010](010_technical_risk_register.md) (the register) ## Context Over two days in August 2026, this repository found **seven** places where it said one -thing and did another. None was a bug in the delivery. All were claims that had outlived +thing and did another. *(An eighth arrived the day after this ADR was accepted, and the +count is left at seven deliberately — see the note at the end of this section.)* None was a bug in the delivery. All were claims that had outlived what they described: - three register entries filed as open with their own stated closing conditions met @@ -33,6 +34,15 @@ The pattern is not carelessness. Each was written accurately and became false wh something else moved. What they share is that **nothing was attached to them that would notice**. +**The eighth, and why the count above stays at seven.** On 2026-08-03 — the day after +this ADR was accepted — PR #211 added a second partner package and found that *eight* +guards were scoped to the first partner by name, so the new one landed exempt from all of +them, including the þing-01 `load_dotenv` prohibition. Register **C-78** records it. The +count above is deliberately not incremented: an ADR that renumbers itself every time the +pattern recurs becomes a changelog, and the argument does not depend on the number. What +the eighth case adds is §2's sharpest form — *a guard's declared scope is part of what +must be mutation-proven, not just its matching.* + ADR-003 already forbids inferring what should be declared. This ADR is that rule applied one level up: **a declaration that nothing validates is an inference with better grammar.** @@ -105,11 +115,24 @@ If a pull request cites a register entry, an issue, a CIC or an ADR as the thing addresses, it updates that artefact **in the same pull request**. Not in a follow-up, not at closeout, not from memory. -**No test enforces this and none is proposed.** It was attempted: no expression reliably -separates *"this entry describes work that is done"* from prose, and a guard that guesses -is one that gets deleted (§3). This clause is therefore a habit, and it is written down -precisely because it is the one rule here that cannot be mechanised — which makes it the -one most likely to lapse. +**No test enforces this in general, though two partial guards exist and are green.** +`tests/test_register_integrity.py` carries +`test_no_open_entry_names_a_closing_artifact_that_already_exists` and +`test_no_open_entry_claims_its_mitigation_has_landed` — both added by S2 in this same +arc. They catch the two mechanisable shapes: an entry naming a file that now exists, and +one whose body says a mitigation landed while its header says Open. + +What resists mechanisation is the general case: no expression reliably separates *"this +entry describes work that is done"* from prose, and a guard that guesses is one that gets +deleted (§3). So the clause is mostly a habit, and it is written down because it is the +rule here least amenable to a test — which makes it the one most likely to lapse. + +*(Corrected 2026-08-03. This paragraph said "none is proposed" and the Alternatives +section called widening the guard "attempted and abandoned", while the widened guard was +already shipping in the same branch. And the guards' reach is genuinely partial: C-15 sat +Open claiming a mitigation had landed via a method that no longer exists, and escaped +`test_no_open_entry_claims_its_mitigation_has_landed` because it phrased the claim +without the em-dash the guard matches. Partial is worth saying; absent was wrong.)* The evidence that it does lapse is this epic's own record. C-71 was fixed on the morning of 2026-08-02 and sat filed open for the rest of the day while eight further stories shipped. @@ -150,5 +173,6 @@ Rejected as a replacement, not as a supplement: a checklist records what to do a and the reasoning is what survives a contributor deciding the rule does not apply to them. Worth adding later, pointing here. -**Widen `test_register_integrity` to catch §5.** Attempted and abandoned — see §5. Recorded -so the next person does not spend the same afternoon on it. +**Widen `test_register_integrity` to catch §5.** Partially done — see §5. Two shapes are +mechanised and green; the general case resists. Recorded so the next person neither +re-attempts the general case nor assumes there is nothing there. diff --git a/docs/CICs/UNFAOPostProcessorManager.md b/docs/CICs/UNFAOPostProcessorManager.md index b1a4d77..0178739 100644 --- a/docs/CICs/UNFAOPostProcessorManager.md +++ b/docs/CICs/UNFAOPostProcessorManager.md @@ -8,11 +8,26 @@ --- +> **Corrected 2026-08-03 — this document named a collaborator the manager has never +> called.** Six statements described enrichment as delegated to `GaulLookupEnricher`, +> one naming the call `GaulLookupEnricher.enrich_dataframe_with_pg_info()`. The manager +> contains **zero** references to it — `tests/test_gaul_lookup_access.py` actively +> asserts its absence — and the sibling CIC has long said *"the manager does not call +> this class."* Two contract documents asserted opposite things about the same call. +> Geography is attached by `contract/historical.py` and `contract/wire/sidecar.py` from +> a lookup the manager loads once per delivery. See register **C-75**. +> +> Five further claims in this file described deleted code and are corrected below: +> a `dotenv` load that no longer happens, a "known gap" in env validation that +> `assert_env_declared` closed, an upload count and file type that were wrong in three +> ways, a selection precondition weaker than `source_selection` enforces, and two +> "incorrect usage" examples for code paths that no longer exist. + ## 1. Purpose > **What is this class for?** -`UNFAOPostProcessorManager` orchestrates the end-to-end postprocessing pipeline that reads VIEWS conflict predictions, enriches them with geographic metadata via the precomputed GAUL lookup (`GaulLookupEnricher`, ADR-011), validates the output schema, and delivers the enriched data to the UN FAO via Appwrite cloud storage. +`UNFAOPostProcessorManager` orchestrates the end-to-end postprocessing pipeline that reads VIEWS conflict predictions, enriches them with geographic metadata from the precomputed GAUL lookup (ADR-011), validates the output schema, and delivers the enriched data to the UN FAO via Appwrite cloud storage. It is the single entrypoint for producing and delivering UN FAO-formatted prediction data. @@ -20,7 +35,7 @@ It is the single entrypoint for producing and delivering UN FAO-formatted predic ## 2. Non-Goals (Explicit Exclusions) -- This class does **not** perform spatial mapping logic — it delegates enrichment to `GaulLookupEnricher` (a merge against the precomputed GAUL lookup) +- This class does **not** perform spatial mapping logic — it reads the precomputed GAUL lookup (`contract/gaul_lookup.load()`) and the artifact builders attach geography from it - This class does **not** train, evaluate, or modify prediction models - This class does **not** define the spatial assignment algorithm - This class does **not** manage shapefile data or geographic reference assets @@ -33,7 +48,7 @@ It is the single entrypoint for producing and delivering UN FAO-formatted predic - Guarantees a 4-stage pipeline: read → transform → validate → save - Guarantees that historical data is sourced from ViewsER via `ViewsDataLoader` - Guarantees that forecast data is sourced from the Appwrite production forecasts bucket -- Guarantees that geographic metadata is added via `GaulLookupEnricher.enrich_dataframe_with_pg_info()` (a cell-id merge against the precomputed lookup) +- Guarantees that geographic metadata is attached from the precomputed lookup — by `contract/historical.py` for the historical artifact and `contract/wire/sidecar.py` for the §5 GAUL sidecar, each a keyed gather on cell id - Guarantees that required metadata columns are validated before upload - Guarantees that both historical and forecast datasets are uploaded to the UN FAO Appwrite bucket with correct metadata (name, loa, type, category) - Logs structural failures before raising them (ADR-008): the config/`loa` guards, the `_validate` gates, and the dataset/`_save` guard all `logger.error`-then-raise (#13 / C-19 resolved); the `delivery/` invariants raise representation-free, with the manager logging context at each call site @@ -45,11 +60,10 @@ It is the single entrypoint for producing and delivering UN FAO-formatted predic - Requires a `PostprocessorPathManager` at initialization pointing to valid model paths - Requires `configs` dict to contain an `ensemble` key naming the source ensemble - Requires environment variables for Appwrite connectivity (endpoint, project ID, API key, bucket/collection IDs) -- Requires the ensemble's `.env` file to be loadable via `dotenv` -- Requires the Appwrite production forecasts bucket to contain at least one file with `category="forecast"` -- Requires the precomputed GAUL lookup parquet to be present so `GaulLookupEnricher` can load it at construction +- Requires the Appwrite production forecasts bucket to contain a **complete run**: a manifest matching `{category: "forecast", type: "sampled_forecast_manifest"}`, a manifest per declared target, and every shard those manifests name. A bucket holding merely *some* `category="forecast"` file raises `SourceSelectionError` (`contract/wire/source_selection.py`) +- Requires the precomputed GAUL lookup parquet to be present; it is read once per delivery via `contract/gaul_lookup.load()` -Assumptions that are not met **must cause failure**, not fallback behavior. **Known gap:** the Appwrite env vars are read via `os.getenv()` without a startup validation — a missing var yields `None`, which is passed to `AppwriteConfig` unchecked rather than failing loud at the boundary (tracked by **C-19-adjacent / #11**; a fail-loud env check is the cheap fix). +Assumptions that are not met **must cause failure**, not fallback behavior. The environment is validated fail-loud: `appwrite_env.assert_env_declared` runs before **both** `AppwriteConfig` constructions and names every missing variable, and an empty string counts as missing. *(This paragraph previously described that as a "known gap" with `os.getenv()` passing `None` through unchecked; C-19 closed it, and `tests/test_env_declaration.py` pins it.)* --- @@ -63,8 +77,7 @@ Assumptions that are not met **must cause failure**, not fallback behavior. **Kn - Downloads data from ViewsER (network I/O) - Downloads forecast data from Appwrite (network I/O) - Writes timestamped parquet files to `data_generated/` directory -- Uploads two parquet files to the UN FAO Appwrite bucket (network I/O) -- Loads `.env` from ensemble path (modifies process environment) +- Uploads to the UN FAO Appwrite bucket (network I/O) — **only when the §11.4 interlock is open**. `product.UPLOAD_ENABLED` is `False` by default, and the sink then makes **zero** store calls. Enabled, a run uploads one parquet per (target, month) — 108 at run-0 — plus the GAUL sidecar parquet, the historical parquet, and a **JSON** run manifest, committed last. Forecast-leg documents carry `{name, category, loa, filename, doc_type, targets}` and no `description`; only the historical artifact carries structured provenance - Logs pipeline progress at INFO/ERROR levels --- @@ -95,7 +108,7 @@ The following **must never** fail silently: ## 7. Boundaries and Interactions **Allowed interactions:** -- Delegates geographic enrichment to `GaulLookupEnricher` (a merge against the precomputed GAUL lookup) +- Reads the precomputed GAUL lookup once and passes it to the artifact builders, which attach geography - Uses `views-pipeline-core` managers for path resolution, data loading, and Appwrite integration - Reads environment variables for external service configuration - Writes to local filesystem and Appwrite cloud storage @@ -132,10 +145,16 @@ manager._save() ## 9. Examples of Incorrect Usage -- **Calling `_transform()` before `_read()`** — datasets will be None, causing AttributeError - **Calling `_save()` without `_validate()`** — may upload incomplete data to partners -- **Accessing `_enricher` directly to bypass the enrichment pipeline** — violates the orchestration boundary - **Hardcoding Appwrite configuration instead of reading from environment** — violates ADR-009 +- **Reaching past the manager into `contract/` to publish** — the sink is driven through + `_ContractStorePort` so the store is one seam; bypassing it also bypasses the + `result.success` check that turns a partial upload into a refusal + +*(Two entries were removed here on 2026-08-03 because they described code that no longer +exists: "calling `_transform()` before `_read()`" — `_transform` is a documented no-op +that cannot raise — and "accessing `_enricher` directly", an attribute removed in #152 +/ C-66.)* --- @@ -157,7 +176,7 @@ The input-integrity guards (S0–S6, epic #51) are representation-free invariant - Partner-specific output formats are **evolving** — the UN FAO schema may change (see C-24, D-06 for schema divergence investigation) - The source of forecast data (Appwrite bucket/collection) is **evolving** — operational configuration - Null validation is **active** (C-01 resolved 2026-06-02) -- The enrichment source is the **precomputed GAUL lookup table** (`GaulLookupEnricher`, ADR-011), as of the Stage 3 swap; the old runtime mapper was **removed** (C-39 / PR #42) — it no longer exists in the repo +- The enrichment source is the **precomputed GAUL lookup table** (`views_postprocessing/data/gaul_lookup.parquet`, ADR-011), as of the Stage 3 swap; the old runtime mapper was **removed** (C-39 / PR #42) — it no longer exists in the repo --- diff --git a/docs/CLONING.md b/docs/CLONING.md index dd1c20c..79844a7 100644 --- a/docs/CLONING.md +++ b/docs/CLONING.md @@ -157,12 +157,19 @@ used to select a retired code path silently (register C-63). **Answer your partner's correction questions before you ship to them, not after a bad delivery.** `docs/operations/correction_procedure.md` is FAO's, and its steps 1–3 and 5 -transfer unchanged — they are contract mechanics, not partner specifics. **Step 4 does -not:** who contacts your partner, through what channel, how fast, and whether they -expect a retraction or a supersession are answers only your partner can give. +transfer as *mechanics* — but **§3's environment variable names do not.** The quarantine +that withdraws a bad run is read by **your consumer**, from a name of its own: +`APPWRITE_UNFAO_QUARANTINED_FILE_IDS` for FAO, +`APPWRITE_CRAFD_QUARANTINED_FILE_IDS` for CRAF'd. Setting the wrong partner's name is a +**silent no-op** — nothing errors and the data keeps serving. Substitute yours before you +need it, not during an incident. + +**Step 4 does not transfer at all:** through what channel, how fast, and whether they +expect a withdrawal or a supersession are answers only your partner can give. This repo shipped run-0 to the UN FAO on 2026-07-27 with that step undecided (register -C-22), and it is still undecided. Do not inherit that. +C-22). It is now half-decided — a named person makes contact, by direct email — and the +rest is with FAO. Settle yours *before* your first delivery, not after. ## Where the reasoning lives diff --git a/docs/operations/correction_procedure.md b/docs/operations/correction_procedure.md index 90fa179..9ffa53c 100644 --- a/docs/operations/correction_procedure.md +++ b/docs/operations/correction_procedure.md @@ -7,9 +7,18 @@ documents first. Where a step needs background, the link is inline and optional. Register **C-22**. Supersedes issue #15, whose procedure described the pre-2026-07 delivery (disk caches, shapefiles) and no longer applies to anything. -> **Status: steps 1–3 and 5 are executable today. Step 4 is not** — nobody has decided -> who contacts the UN FAO, or whether FAO expects a retraction or a supersession. Those -> are the operator's calls and are stated in §4 exactly as they need to be answered. +> **Status: steps 1–3 and 5 are executable today. Step 4 is executable but incomplete.** +> *Who* contacts the UN FAO is decided — Simon Polichinel von der Maase, by direct email +> (§4). What is **not** decided is whether those are the right recipients, what +> notification period FAO expects, and whether FAO wants a withdrawal or a supersession. +> Those are put to FAO as Pre-Release Note 07, Topic B. **Follow §4 as written rather +> than improvising** — an inconsistent first message to a partner is harder to correct +> than a slow one. +> +> *(Corrected 2026-08-03: this banner said nobody had decided who makes contact, while +> §4 twenty lines below already named the responsible person. The guard in +> `tests/test_doc_accuracy.py` asserts §4's sentence exists and never read the banner +> beside it.)* --- @@ -27,18 +36,35 @@ understand before touching anything. ## 1. Which deliveries are affected? -Two fields identify a delivery, and both are on it by construction: +Two fields identify a delivery — and **they are not on the same artifact.** Know this +before you try to join them, because you cannot. -| field | what it answers | where it lives | -|---|---|---| -| `run_id` | *which run* | the manifest, every shard's header, and each store document's filename | -| `lookup_version` | *which GAUL build produced its geography* | the historical artifact's store-document `description` (a compact JSON provenance record) | +| field | what it answers | where it lives | where it does NOT | +|---|---|---|---| +| `run_id` | *which run* | the manifest, every shard's header, each store document's filename | the historical artifact (its filename is `historical_dataset_.parquet`) | +| `lookup_version` | *which GAUL build produced its geography* | the historical artifact's store-document `description` (a compact JSON provenance record) | the manifest, the shards, **and the GAUL sidecar** | `lookup_version` has the form `land_gaul@f74d3b2b` — region, then the digest of the views-datafactory ingestion the lookup was built from. **It cannot silently be `"unknown"`**: since register C-60 the reader raises rather than degrading, so a stamp you can read is a stamp you can trust. +**Two consequences you will hit immediately.** + +1. **There is no key joining a `run_id` to a `lookup_version`.** The historical artifact + carries the geography stamp and no run id; the manifest carries the run id and no + geography stamp. In practice you correlate them by **upload timestamp proximity** + within the bucket. Say so in your incident notes rather than implying a join. +2. **The forecast leg carries no geography stamp at all.** The §5 GAUL sidecar has no + version field (ADR-014 §4 defers stamping it to the next ADR-013 version bump). So a + *geography* fault can be scoped precisely on the historical leg and only by timestamp + on the forecast leg. If that is the fault you have, quarantine generously — the cost + of withdrawing one run too many is a re-publish; the cost of leaving one is FAO + serving wrong geography. + +*(Corrected 2026-08-03: this section said both fields were "on it by construction", +which reads as though they sit on one artifact. They never have.)* + **To enumerate affected runs:** 1. If the fault is in **geography** (a wrong country, admin unit or coordinate), the @@ -50,8 +76,10 @@ you can read is a stamp you can trust. `expected_cell_count` and the provenance record's `actual_cell_count` bound it without opening a single parquet. -Do this before touching the store. A correction whose scope you have not established -is a second incident. +Establish scope before you *correct* — a correction whose scope you have not +established is a second incident. **But withdraw first** (§3): quarantine is reversible +and costs seconds, and scoping while wrong data serves is the expensive order. Withdraw +generously, then narrow. --- @@ -64,10 +92,20 @@ re-running changes the thing you are diagnosing. pytest -q tests/test_gaul_lookup_fidelity.py ``` -26 tests. The always-on half checks the committed lookup for key uniqueness, region-set -equality, coordinate correctness, and the absence of nulls and `-1` sentinels. With a -views-datafactory checkout present (`VIEWS_DATAFACTORY`), the second half compares all -seven GAUL columns against the producer's own parquets. +24 tests. **What runs without a views-datafactory checkout** — key uniqueness, cell +count, exclusion-set disjointness, coordinate correctness, and the absence of nulls and +`-1` sentinels, all against the committed lookup. + +**What does NOT run without one**, and this matters if you are on a laptop: the +comparison of all seven GAUL columns against the producer's own parquets, region-set +equality, the coordinate-dtype wire-stability check, and the per-cell coordinate-formula +check — four tests in all. The dtype one touches only the committed lookup and is gated +more strictly than it needs to be. Those tests **skip**, they do not fail. Check the pytest summary +line for `skipped` before concluding you have verified the geography — a green run with +skips is a weaker statement than a green run without them. + +*(Corrected 2026-08-03: this said "26 tests" — a count that was never true — and listed +region-set equality in the always-on half when it is gated.)* **If those pass and a value is still wrong, the fault is upstream, not here.** That distinction is register C-43's and it is load-bearing: this repository is verified to @@ -80,21 +118,68 @@ defect; fix the producer and rebuild. ## 3. Correct on the wire -**The intended policy is WITHDRAWAL. What is implemented is SUPERSESSION.** That gap is -deliberate, recorded, and you need to know about it before you act. +### DO THIS FIRST: withdraw the bad run. It takes one environment variable. + +**Corrected 2026-08-03.** This section previously said withdrawal had no mechanism and +there was no rollback. That was wrong, and following it cost hours: it sent you to fix +the cause and republish an entire corrected run while the wrong data stayed live. + +views-faoapi ships an operator quarantine (`f1a59bf`, on its `main`, +`managers/prediction/quarantine.py`). It is a comma-separated list of **store-document +file-ids**; selection drops anything on it *before* choosing. + +**⚠ A run has TWO selection entry points, and quarantining one does not withdraw the other.** (A run is ~112 files — §0. What matters here is that the consumer reaches them through two independent doors.) + +| leg | what to quarantine | how the consumer selects it | +|---|---|---| +| forecast | the run **manifest**'s file-id | newest manifest, then the shards it names | +| historical actuals | the **historical parquet**'s own file-id | newest document with `category="historical"` — **it never looks at the manifest** | + +``` +APPWRITE_UNFAO_QUARANTINED_FILE_IDS=, +``` + +Quarantining only the manifest leaves the historical artifact serving. That is the wrong +way round for a **geography** fault: `lookup_version` lives *only* on the historical +document (§1), so the leg you can identify precisely is the leg you would have left +live. When in doubt, quarantine both — the cost of withdrawing one document too many is +a re-publish. + +Both legs go through the same filter (`prediction/manager.py:145-156`, inside +`get_predictions_by_metadata` — which the forecast path reaches via `get_latest_file_id` +and the historical path via `get_latest_provenance`), so one variable covers both. Read at selection time — **no redeploy**. Nothing is deleted, so it is reversible: +unset the variable and the run is selectable again. Whitespace around entries is +tolerated; quotes are not stripped, so do not quote the ids. + +There is a matching allowlist, `APPWRITE_UNFAO_APPROVED_FILE_IDS`, which restricts +selection to explicitly approved files (unset or empty = unrestricted). + +**These names are FAO's.** Each partner's consumer reads its own — CRAF'd's is +`APPWRITE_CRAFD_QUARANTINED_FILE_IDS`. Setting the wrong partner's variable is a +**silent no-op**: nothing errors, and the data keeps serving. -- **Intended** (operator decision, 2026-08-02): a delivery found to be materially wrong - should be *removed or blocked* so it can no longer be served. Data known to be wrong - should not stay retrievable through the production interface. -- **In force**: supersession. The manifest-last commit ordering means a run is replaced - by publishing a *new complete run*; the old one stays in the bucket and remains - retrievable by a client that asks for it specifically. +**Withdraw first, then diagnose.** Steps 1–4 below are the correction; the quarantine is +the stop-the-bleeding move that precedes them, and it is what makes the rest unhurried. -Supersession is in force **because it is what the wire does**, not because it was chosen. -Withdrawal has no mechanism today: it needs an ADR-013 amendment and matching work in -views-faoapi. Put to FAO as **Pre-Release Note 07, Decision Point B.2**, which also asks -whether they have an audit or reproducibility requirement that would argue *against* -withdrawal — that answer decides whether the amendment is worth building. +### Then correct: withdrawal vs supersession + +- **Withdrawal** — the intended policy (operator decision, 2026-08-02): data known to be + wrong should not stay retrievable. **Available now**, via the quarantine above. +- **Supersession** — what the *wire itself* does: manifest-last ordering means a run is + replaced by publishing a new complete run, and the old one stays in the bucket. + +The two compose: quarantine withdraws immediately, publishing supersedes durably. Put to +FAO as **Pre-Release Note 07, Decision Point B.2**, which asks whether they have an audit +or reproducibility requirement arguing *against* withdrawal. + +> ⚠ **Note 07 Topic B.2 needs correcting before it goes to FAO.** It presents withdrawal +> as *"Not available today. Requires a change to the delivery contract … and +> corresponding work in the API layer."* That is false — see above. The decision FAO is +> being asked to make is real, but the cost framing given to them is not. + +### Publishing the correction + +With the bad run withdrawn, this part is unhurried: 1. Fix the cause — the lookup, the producer, or the code — and land it. 2. Rebuild any affected artifact. For the lookup: @@ -109,13 +194,19 @@ withdrawal — that answer decides whether the amendment is worth building. ### What the consumer will actually pick up — read this before assuming -views-faoapi resolves **the newest manifest matching a broad filter** -(`wire/source_selection.py`). So a corrected run is picked up because it is *newer*, not -because it is *correct*. Two consequences: +views-faoapi resolves **the newest manifest matching a broad filter** — its own +`src/views_faoapi/managers/prediction/manager.py::get_latest_manifest`, minus anything +quarantined. So a corrected run is picked up because it is *newer*, not because it is +*correct*. Two consequences: + +*(Corrected 2026-08-03: this cited `wire/source_selection.py`. That file is **this** +repo's Hop-A **inbound** selector — how we consume pipeline-core forecasts — not how FAO +selects. views-faoapi has no `wire/` package at all.)* - **Publishing the correction is what switches the consumer over.** There is no - "activate" step and no rollback — republishing the old run would mean publishing it - again under a new id. + "activate" step in the *publishing* path — but there IS a rollback: quarantine the + bad run's documents (above) and the consumer falls back to the previous ones. + Republishing an old run, by contrast, would mean publishing it again under a new id. - **Recency-based selection is register C-73**, open, and fixed upstream in views-pipeline-core 3.0.0 but not yet taken here. **#133** would have the manifest declare `{maturity, source, required-schema-version}` so a consumer could select on @@ -158,7 +249,9 @@ an inconsistent first message to a partner is harder to correct than a slow one. ## 5. Preserve the evidence -A retraction that destroys the evidence makes the post-mortem impossible. Keep, at +A withdrawal that destroys the evidence makes the post-mortem impossible — which is +why quarantine is the right instrument: it removes the run from *selection* without +removing it from the bucket. Keep, at minimum: - the superseded run's `run_id`, its manifest, and its provenance record @@ -177,6 +270,8 @@ leaves the hole. `docs/ADRs/013_sampled_forecast_wire_contract.md` §4 (commit ordering), §11 (transition rules) · `views_postprocessing/delivery/provenance.py` (the fields a delivery carries) · -`views_postprocessing/contract/wire/source_selection.py` (how the consumer chooses) · +`views_postprocessing/contract/wire/source_selection.py` (how **this repo** chooses its +Hop-A input — *not* how FAO chooses; that is views-faoapi's +`managers/prediction/manager.py`) · register **C-22**, **C-43**, **C-60**, **C-73** · issues **#15**, **#131**, **#133**, views-datafactory **#387** diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 787473f..d54c5ce 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -5,8 +5,8 @@ | Project | views-postprocessing | | Owner | Dylan Pinheiro / PRIO MD&D Team | | Last Updated | 2026-08-03 | -| Total Concerns | 79 | -| Open Concerns | 19 | +| Total Concerns | 82 | +| Open Concerns | 22 | | Resolved Concerns | 60 | --- @@ -41,7 +41,7 @@ covered a single open entry (see Historical clusters below). **Root cause:** a family of entries whose entire risk statement was "unverified until the first global run" — all keyed to one event, which occurred **2026-07-27**. **Entries:** C-43 (the survivor), C-30 + C-34 (merged, discharged), C-32 (discharged), C-25 (residual), D-12 and D-09 (deferral conditions) **Highest tier:** 2 (C-43) -**Fix strategy:** one post-run-0 verification pass against producer run `rusty_bucket_forecasting_20260727_095355` — issue **#131 q1**. +**Fix strategy:** one post-run-0 verification pass against producer run `rusty_bucket_forecasting_20260727_095355` — issue **#131 q1**, **CLOSED 2026-07-31**. C-43 resolved 2026-08-02; this cluster is discharged. **Resolution scope:** Full for C-30/C-32/C-34. **Partial for C-43 — the finding that matters.** Run-0 discharged the *availability* half of this cluster (the path runs, memory is bounded at 5.6 GB, coverage is proven at 64,742 cells). It discharged **none of the correctness half**, because proving the path *runs* at scale was never what C-43 asked for. **C-43 now stands alone and un-gated, with delivered data in the partner store.** ### Cluster I: Governance-artifact drift @@ -160,13 +160,25 @@ that indexes only deleted code is noise. | Tier | 3 | | Source | `repo-assimilation` (2026-06-02) | | Trigger | When `views-pipeline-core` updates its dependency tree (e.g., drops `geopandas` or `joblib`), verify that this package's imports still resolve | -| Location | `pyproject.toml:11-15`; `views_postprocessing/unfao/managers/unfao.py:14`, `unfao/enrichment.py:26`, `unfao/extraction.py:26` | +| Location | `pyproject.toml` (the dependency block); the partner managers, which are the only modules importing `views_pipeline_core`. *(This row previously cited `unfao/enrichment.py` and `unfao/extraction.py` — both moved or deleted by #151/#153.)* | `mapping.py` directly imports `geopandas`, `shapely`, `numpy`, `pandas`, `joblib`, and `multiprocessing`. `unfao.py` directly imports `pandas`, `polars`, and `python-dotenv`. Only `views-pipeline-core` and `cachetools` are declared in `pyproject.toml`. The undeclared dependencies presumably arrive transitively via `views-pipeline-core`, but this coupling is implicit and fragile. If the upstream package refactors its dependency tree, this package will break with `ImportError` at install time. **Update 2026-06-24 (narrowed):** the `mapping.py` dimension is gone (C-39 — the `geopandas`/`shapely`/`joblib`/`multiprocessing` imports were deleted; `cachetools` dropped from `pyproject.toml`). Residual: `unfao.py` imports `pandas`/`polars`/`python-dotenv` undeclared, arriving transitively via `views-pipeline-core` (which *is* declared). Much smaller surface (Tier 4-ish); consider resolving outright if the transitive-via-pipeline-core guarantee is deemed sufficient. -**Update 2026-08-01 (`falsify`) — a SECOND undeclared dependency, and this one's transitive path is about to disappear.** `appwrite` is used in this repo (`contract/launch_config.py`, `unfao/managers/unfao.py`) and declared in **no** manifest — it arrives transitively via `views-pipeline-core`, exactly as `pandas` does. What makes it different from the pandas residual: **views-pipeline-core is making `appwrite` an optional extra** (their **#345**, on CRP grounds — three repos that never mention Appwrite currently install its SDK). **When that lands, the transitive path disappears and this repo breaks at import.** +**Update 2026-08-01 (`falsify`) — a SECOND undeclared dependency, and this one's transitive path is about to disappear.** `appwrite` is used in this repo (`contract/launch_config.py`, `unfao/managers/unfao.py`) and declared in **no** manifest — it arrives transitively via `views-pipeline-core`, exactly as `pandas` does. What makes it different from the pandas residual: **views-pipeline-core is making `appwrite` an optional extra** (their **#345**, on CRP grounds — three repos that never mention Appwrite currently install its SDK). **When that lands, the transitive path disappears.** + +**⚠ Corrected 2026-08-03 — the trigger has FIRED and the stated failure mode was wrong.** +pipeline-core **#345 is CLOSED**, and 3.0.0 does make `appwrite` an optional extra. But +this repository contains **zero** direct Appwrite SDK imports (`grep -rn '^\s*\(from\|import\) appwrite' views_postprocessing/` → 0); +`contract/launch_config.py` mentions the word once, in a docstring naming its sibling +`appwrite_env`. So "this repo breaks at import" was never true of *our* imports. + +The real and still-live risk is one level out: **pipeline-core's own `DatastoreModule` +imports the SDK unguarded**, so bumping to 3.0.0 without declaring the `appwrite` extra +breaks the delivery at import — inside a dependency, which is harder to diagnose than a +break in our own code. That is a precondition on the C-44 bump, and it belongs there as +much as here. So this entry is no longer "Tier 4-ish, resolve if the transitive guarantee is deemed sufficient" — the guarantee is being **withdrawn upstream, deliberately**. Fix is one line: declare `appwrite` in `pyproject.toml`, or depend on `views-pipeline-core[appwrite]`. Relayed in **#172**; registered here rather than as a new entry because it is the same problem type at a new location. **New trigger: before views-pipeline-core#345 lands.** @@ -196,13 +208,13 @@ So this entry is no longer "Tier 4-ish, resolve if the transitive guarantee is d | Tier | 3 | | Source | `expert-review` (2026-06-02), `falsification-audit` (2026-06-02) | | Trigger | When wiring pipeline-core #245's structured metadata field, or when adding/removing a provenance key — verify the closed keyset in `delivery/provenance.py` and the `DESCRIPTION_MAX` bound still hold, and that the carrier is no longer free-text `description` | -| Location | `views_postprocessing/delivery/provenance.py`; `views_postprocessing/unfao/managers/unfao.py:559-578` (legacy `_save` uploads), `:593-604` (`_historical_frame_description`), `:619` (`_delivery_description`) | +| Location | `views_postprocessing/delivery/provenance.py` (`build_provenance`); `_historical_frame_description` in each partner's manager (the only caller); `contract/wire/sink.py` (the forecast leg, which attaches none). Symbols rather than line numbers — the earlier row's three line ranges were all past end-of-file. | Both `dsm.upload_data()` calls in `_save()` carry metadata: `name`, `loa`, `type`, `targets`, `description`, `category`. The `description` field was updated from a hardcoded test string to an enrichment timestamp (`"Enriched with geographic metadata on {timestamp}"`). However, broader enrichment provenance is still missing: no shapefile version/hash, no enrichment error count, no unmapped cell count. The consumer cannot verify which shapefile version produced their data or whether any errors occurred during enrichment. Tier recalibrated from 4 to 3 during falsification audit (2026-06-02): the missing provenance affects the partner's ability to audit data quality. -**Mitigation landed (S5, 2026-06-26, `sprint/fao-input-integrity`):** a representation-free `delivery/provenance.py` (`build_provenance`) assembles structured provenance — `lookup_version`, `region`, `expected_cell_count`, `actual_cell_count`, `unmapped_count` — sourced from the enricher + S1 coverage + a new `extraction.unmapped_cell_count` seam (nothing hardcoded). Both `_save` uploads now carry it via `_delivery_description`. **Carrier constraint:** pipeline-core's `upload_data` exposes **no structured field** — only free-text `description` — so the dict is JSON-encoded into `description` behind a human prefix for now. A dedicated metadata field is requested upstream (**pipeline-core #245**); when it lands, only the manager's attach step changes (the provenance shape is already representation-free). `fill_count` is omitted until a fabricated-value count is available (cf. C-26). Residual is now just the carrier abuse, tracked by #245. +**Mitigation landed (S5, 2026-06-26, `sprint/fao-input-integrity`):** a representation-free `delivery/provenance.py` (`build_provenance`) assembles structured provenance — `lookup_version`, `region`, `expected_cell_count`, `actual_cell_count`, `unmapped_count` — sourced from the enricher + S1 coverage + a new `extraction.unmapped_cell_count` seam (nothing hardcoded). The historical upload carries it via `_historical_frame_description`. *(This sentence said "Both `_save` uploads now carry it via `_delivery_description`" — that method was deleted with the legacy path in #149, and there is now only one provenance-carrying upload: the forecast leg's shards carry `{name, category, loa}` and no `description` at all, which is the residual below.)* **Carrier constraint:** pipeline-core's `upload_data` exposes **no structured field** — only free-text `description` — so the dict is JSON-encoded into `description` behind a human prefix for now. A dedicated metadata field is requested upstream (**pipeline-core #245**); when it lands, only the manager's attach step changes (the provenance shape is already representation-free). `fill_count` is omitted until a fabricated-value count is available (cf. C-26). Residual is now just the carrier abuse, tracked by #245. See also C-14 (stale cache without version tracking), C-22 (no post-delivery correction process), C-26 (fabricated zeros — the eventual `fill_count` source). @@ -216,7 +228,7 @@ See also C-14 (stale cache without version tracking), C-22 (no post-delivery cor | Tier | 3 | | Source | `falsification-audit` (2026-06-02) | | Trigger | When views-faoapi implements the Release-Note-01 Topic-C renaming layer — verify this repo's column names stay **unchanged** (faoapi's `_METADATA_COLS` validation depends on them) and that the rename lands consumer-side only. Take no action here otherwise. | -| Location | `views_postprocessing/unfao/managers/unfao.py:277` (`filter_cols`), `unfao/gaul_schema.py` (`METADATA_COLS`), FAO Release Note 01 `topic_c.tex` | +| Location | `contract/gaul_schema.py` (`METADATA_COLS`, the declared 9-column contract); `contract/historical.py` and `contract/wire/sidecar.py` (which project it); FAO Release Note 01 `topic_c.tex`. *(This row previously cited `unfao.py:277 (filter_cols)` — no such symbol exists anywhere in the package, and `unfao/gaul_schema.py` moved to `contract/` in #153.)* | The FAO API contract (Release Note 01, Topic C, confirmed and locked) specifies: UN M49 country codes, `ADM1_CODE`/`ADM1_NAME`/`ADM2_CODE`/`ADM2_NAME` for admin fields, and `lat`/`lon` for coordinates. The postprocessor's `filter_cols` uses: `country_iso_a3` (ISO Alpha-3), `admin1_gaul1_code`/`admin1_gaul1_name`/`admin2_gaul2_code`/`admin2_gaul2_name`, and `pg_xcoord`/`pg_ycoord`. Three of four data categories (country ID, admin fields, coordinates) use different naming conventions from the locked contract. @@ -621,6 +633,94 @@ Cross-refs: **C-59** and **C-61** (RESOLVED — the invariant block this sits be +--- + +### C-80: The doc-accuracy scan exempts ADRs and CICs — the two artifact classes that define the contracts + +| Field | Value | +|-------|-------| +| ID | C-80 | +| Tier | 2 — structural, with a demonstrated failure. A CIC is what a contributor reads before changing a class; an ADR is what a consumer reads before building against the wire. Both were free to describe deleted code indefinitely, and did. | +| Source | `code-review max` (2026-08-03) — development→main sync audit | +| Trigger | When the next module is moved or deleted, check whether any ADR or CIC names it. The deleted-symbol regex will not tell you. #153 moved seven modules out of `unfao/` and the ADRs still cite the old paths. | +| Owner | Whoever next extends `tests/test_doc_accuracy.py`. It is a scope change plus a decision about how to exempt genuine history. | +| Location | `tests/test_doc_accuracy.py` — `_living_docs()` and `_link_checked_docs()` | + +`_living_docs()` returns `README.md`, `docs/architecture/*.md`, and package `README.md`s. **`docs/ADRs/` and `docs/CICs/` are outside it**, deliberately — an ADR legitimately records superseded designs, and a scan that fires on history gets deleted (§3). The exemption is right in principle and far too wide in practice. + +**What it cost, measured in this sync.** `docs/CICs/UNFAOPostProcessorManager.md` named `GaulLookupEnricher` as the manager's enrichment collaborator in **six** places, one of them a specific call — while the manager contains zero references and `tests/test_gaul_lookup_access.py` actively asserts its absence. The sibling CIC said the opposite in plain words. Two contract documents contradicted each other about the same call, and nothing could see it. Five further claims in the same file described a `dotenv` load that does not happen, an env-validation "known gap" that C-19 closed, an upload count wrong in three ways, and two "incorrect usage" examples for code deleted in #149/#152. ADR-013 still cites `unfao/wire/`, `unfao/product.py` and `unfao/launch_config.py`, all moved in #153. + +**The exemption is not understood by the people writing under it.** `docs/CICs/UNFAOPostProcessorManager.md` carries a `legacy-ok` marker — the line-scoped opt-out from a scan that never reaches that file. Its author believed they were suppressing a guard that was not looking. + +**A second, narrower hole in the same file.** `test_internal_doc_links_resolve` follows only markdown `](...)` links. Every path written as prose in backticks — which is how this repository writes paths almost everywhere — is unchecked. That is why the stale `unfao/...` references survived a dedicated sweep (S11) and were still being found two epics later. + +*Not proposed as a fix here:* pointing the existing regex at ADRs would fire on every historical passage and be reverted within a day. The shape that works is what §3 already recommends — check the **claim**, not the vocabulary: for CICs, that every collaborator named is actually referenced by the class (the negative form already exists at `test_gaul_lookup_access.py:156`); for backticked paths, that a path-shaped token which looks like a repo path resolves, with an opt-out for history. + +Cross-refs: **C-74** (a guard narrower than its declared surface), **C-78** (a guard whose declared scope missed a package), **C-67** (ADR-012 drift, which *is* covered and was caught), ADR-014 §1–§3, #211. + +--- + +### C-81: What actually gates `main` is weaker than it looks — CI verifies 17 fewer tests than local, and nothing requires it to pass + +| Field | Value | +|-------|-------| +| ID | C-81 | +| Tier | 2 — the guards this arc built to catch cross-repo drift do not run where drift happens, and the branch they protect has no required check. Both halves are structural and both have fired-in-practice evidence. | +| Source | `code-review max` (2026-08-03) — development→main sync audit | +| Trigger | **Coverage half:** when the Appwrite Seam Contract registry next moves — it moved twice on 2026-08-03 alone — nothing in CI will notice; only a maintainer running the suite locally will. **Enforcement half:** the first time someone merges a red PR to `main`. | +| Owner | Simon — both halves need operator action. The coverage half needs a token for two private repositories; the enforcement half is a GitHub console/ruleset change. Neither is engineering work. | +| Location | `.github/workflows/run_pytest.yml`; the `protect_main` ruleset; `tests/conftest.py::sibling_repo` | + +**Coverage.** Measured in an isolated clone, not estimated — **402 collected in every run**, so the whole delta is skips: + +| environment | result | +|---|---| +| local, all siblings present | 362 passed / 40 xfailed / **0 skipped** | +| CI as it was | 347 passed / **17 skipped** / 38 xfailed | +| CI with the views-crafdapi checkout added | 348 passed / **16 skipped** / 38 xfailed | + +*(**This table was wrong twice, and the second time it refuted itself.** Draft one said 361/40 and "same 401" — measured before the same change added a test. Draft two fixed the collected figure to 402 and did not re-derive the rows, so both rows summed to 401 beside an assertion that 402 was collected. The cause of the second error is worth recording: the measurement was taken on a `git clone` of the branch, and a clone carries **committed** state — the new tests were still uncommitted in the working tree. Measuring a claim about your own change requires applying your own change. This is the entry about miscounted tests.)* `sibling_repo` resolves `$VIEWS_` else `../`; in a one-repo checkout neither exists and the tests skip. Skipping is correct behaviour — a missing sibling *is* normal — but the consequence is that **CI verifies strictly less than a developer's laptop, precisely on the assertions that cross a repository boundary.** + +Nine of the seventeen are **new in this arc**, including both registry-drift detectors (pinned edition, commit-reachable-from-`main`) for both partners. Those detectors have a demonstrated drift rate: they fired **twice on 2026-08-03**, hours apart. A detector for a fault that recurs twice in a day, running only on one machine, is most of the way to not existing. + +Where each sibling stands, after trying them: +- **views-crafdapi** — public, its check reads source text. **Now checked out in CI**, recovering **one** test: the cross-seam consumer-document-name pin for CRAF'd. +- **views-datafactory** — public, but its eight tests need the producer's raw GAUL parquets, which are **not in its git repository**. Checking it out converts an honest skip into a `FileNotFoundError`; tried and reverted. +- **views-appwrite**, **views-faoapi** — **private**. The most valuable checks live here. Closing this needs a token in CI. + +**Enforcement.** `main` is **not branch-protected**: `gh api .../branches/main/protection` returns `404 Branch not protected`, and `gh api .../rules/branches/main` returns `[]`. The `protect_main` ruleset exists and is `active`, but its `ref_name` include-list is **empty**, so it matches nothing — and it declares no `required_status_checks` rule in any case. **A red `Run Pytest` would not block a merge to `main`.** This repository's own `tests/test_falsification_campaign_4_1.py` carries the question as an unverifiable xfail probe; it is verifiable through the API, and the answer is no. + +The two compound: a suite that checks less than you think, and no requirement that even that much passes. Neither is caused by this sync — both are pre-existing — but this sync is the first time `main` receives an epic whose value is largely the guards themselves. + +Cross-refs: **C-46** and **C-57** (both RESOLVED; this is the residual each recorded as *"a CI-cost and cross-repo-coupling decision"* and *"worth deciding once for both"* — it now has a live home and a concrete answer per sibling), **C-80** (the other verification gap found in the same audit), #188. + +--- + +### C-82: Governance-artifact prose carries numbers and statuses that nothing checks + +| Field | Value | +|-------|-------| +| ID | C-82 | +| Tier | 3 — no delivery is affected, but these are the artifacts people plan from. One instance materially under-scopes a planned dependency bump. | +| Source | `code-review max` (2026-08-03) — development→main sync audit | +| Trigger | When the pipeline-core 3.0.0 bump (C-44) is scoped from Cluster M's summary rather than from C-72's body, or when anyone counts on a test-count or issue-state stated in the register. | +| Owner | Whoever runs the next `review-rr` pass; this is curation, not engineering. | +| Location | `reports/technical_risk_register.md` (Clusters I, J, M; D-09, D-11); `docs/CICs/*.md` front matter | + +`tests/test_register_integrity.py` checks structure — header counts, section placement, reference resolution — and **no prose at all**. Roughly twenty-five statements drift beneath it. + +**The one that would change a decision.** Cluster M declares resolution *"Full for … C-72 …"* at the pipeline-core 3.0.0 bump, while C-72's own body says its fix is gated on pipeline-core **#280** (open), **changes delivered wire bytes**, and requires a coordinated three-repo re-vendor of the ADR-013 §10 golden fixture. Someone planning that bump from the cluster summary under-scopes it badly. Cluster M's heading also says six entries where its body says five. + +**Self-contradiction elsewhere.** Cluster I still argues that *"there is no equivalent for the register — a small `tests/test_register_integrity.py` … would make this class self-detecting"*; that file exists, has ten green tests, and is cited elsewhere in the same document. Cluster J names issue **#15** as its fix strategy; #15 is closed and superseded by `docs/operations/correction_procedure.md`. D-11 says a branch *"currently has no scheduled deletion PR"* two paragraphs after recording that it was deleted. D-09's `Status` row reads *"Open … after delivery"* directly above prose recording the deferral expired on 2026-07-31. + +**Numbers.** The `test_gaul_lookup_fidelity.py` count appears as **26** twice in the register and as **18** twice more including `test_register_integrity.py`'s own docstring; the actual is **24**, and 26 was never true — it was written when the file held 24. Also *"40 ADR-013 guard tests"* (39) and *"`test_enrichment.py`, 16"* (39). + +**CIC front matter.** `GaulLookupEnricher.md` says *Last reviewed 2026-06-18* and `UNFAOPostProcessorManager.md` *2026-06-02*, while both bodies carry 2026-08 content. A reader calibrating trust from the header calibrates it wrong in the safe direction, which is lucky rather than designed. + +*The general fix is C-80's, not a re-count:* prose that states a number is a claim, and a claim needs a check. Where a number cannot be checked, the honest move is to state the command that produces it — which is what C-33 was forced into after its measurement was wrong five times. + +Cross-refs: **C-80** (the same disease in ADRs and CICs, and the mechanism that would catch both), **C-72** and **C-44** (the bump this mis-scopes), **C-33** (the worked example of publishing the command instead of the result), ADR-014 §1. + --- ### C-79: `_ContractStorePort.upload`'s result check is called "the whole mechanism" and has no test, and it fails open @@ -693,7 +793,7 @@ Three maintainer-raised intents, assessed and **deliberately deferred** — all **Re-open trigger:** Run 0 verified AND retention owner named — then sequence 2→3 (or 2 alone) as an infrastructure epic, and 1 whenever wire churn is calm. See also C-40 (the migration this rides on), ADR-013 §8. -**Status 2026-07-31 (review-rr — trigger HALF fired):** **Run 0 delivered** on 2026-07-27 (first FAO global-land forecast, frame-native, no OOM) — but it is **delivered, not yet verified**: issue #131 q1 (manifest integrity, sidecar/parity, 3 targets × 36 months, coverage gate on both frames) is still open, and #131 also surfaced a liveness dialect gap on the `unfao_delivery` forecast surface. **The retention owner is still unnamed** (ADR-013 §3.5 records the duty as OPEN). Both halves must hold before this re-opens, so it stays deferred — but it is now one open verification away, not one delivery away. Note that intent 2 (move `production_forecasts` off Appwrite) and the unnamed retention owner compound: run-0 added ~110 objects in a single run to a store with no retention policy. +**Status 2026-08-03 (both halves re-checked): #131 is CLOSED (2026-07-31), so the verification half of this trigger HAS fired.** The text below was written the day it closed and was already stale; it is corrected rather than deleted because the deferral it holds shut is a live decision. **Run 0 delivered** on 2026-07-27 (first FAO global-land forecast, frame-native, no OOM) and its integrity verification is closed — and #131 also surfaced a liveness dialect gap on the `unfao_delivery` forecast surface. **The retention owner is still unnamed** (ADR-013 §3.5 records the duty as OPEN), and that is now the *only* thing holding this deferral shut. Both halves must hold before it re-opens; one of the two now does. **Naming a retention owner re-opens D-12** — that is an operator decision, not engineering work. Note that intent 2 (move `production_forecasts` off Appwrite) and the unnamed retention owner compound: run-0 added ~110 objects in a single run to a store with no retention policy. --- @@ -800,14 +900,16 @@ The document states both verbatim and instructs the reader to stop and ask rathe `docs/CLONING.md` carries the same warning forward: a clone should answer its partner's correction questions **before** first delivery. This repo shipped run-0 on 2026-07-27 with that step undecided, and it still is. | | Tier | 3 | | Source | `falsification-audit` (2026-06-02) | -| Trigger | When the run-0 integrity verification (#131 q1) or any FAO/faoapi query surfaces a suspect delivered value — follow the correction procedure; **issue #15 must produce one first.** Re-check at every subsequent delivery until it exists. | +| Trigger | When any FAO/faoapi query surfaces a suspect delivered value — follow `docs/operations/correction_procedure.md`, **withdrawing first** via views-faoapi's quarantine before diagnosing. *(This row previously said "issue #15 must produce one first" and named #131 q1 as a precondition. #15 is CLOSED and superseded by the procedure; #131 closed 2026-07-31. The procedure exists — the trigger is now the incident, not the paperwork.)* | | Location | `views_postprocessing/unfao/managers/unfao.py:442-494` (`_save_contract`), `:518-578` (legacy `_save`); issue #15 (the undocumented procedure) | The delivery chain has four stages beyond the code: Appwrite bucket → UN FAO download → FAO systems → operational decisions. When an error is discovered post-delivery, correction requires clearing cache, re-running, re-uploading, notifying FAO, and FAO retracting old data. Steps 3-5 have no documented procedure. Part of Cluster B (operational impact dimension). See also C-14 (RESOLVED — mapper-era cache), C-15. -**Update 2026-07-31 (review-rr — the conditional is spent):** this entry was written conditionally — "*if* wrong data ever reaches FAO." **Run-0 delivered on 2026-07-27** (108 arrow shards + sidecar + manifest to `unfao_bucket`, plus 28,356,996 historical rows at 64,742 cells), and its integrity verification is still open (#131 q1). There is now delivered, unverified data in the partner's store and still no documented correction/recall procedure. Tier held at 3 (process gap, no code defect), but this is the acute member of Cluster J — **issue #15 is now the blocking artifact, not a nice-to-have.** +**Update 2026-07-31 (review-rr — the conditional is spent):** this entry was written conditionally — "*if* wrong data ever reaches FAO." **Run-0 delivered on 2026-07-27** (108 arrow shards + sidecar + manifest to `unfao_bucket`, plus 28,356,996 historical rows at 64,742 cells). There is delivered data in the partner's store. + +**Corrected 2026-08-03.** The paragraph above ended *"its integrity verification is still open (#131 q1) … still no documented correction/recall procedure … issue #15 is now the blocking artifact."* All three are spent: **#131 closed 2026-07-31**, the procedure landed as `docs/operations/correction_procedure.md`, and **#15 is closed and superseded by it**. What remains genuinely open is narrower and is in the procedure's §4: FAO has not yet answered who else to notify, in what period, and whether they want withdrawal or supersession (Pre-Release Note 07, Topic B). --- @@ -947,7 +1049,7 @@ The right axis was **exact equality on string constants**, not statement shape. **Gated, and honestly so.** The checks need a views-appwrite checkout and skip without one, naming `VIEWS_APPWRITE` and the conventional sibling path so a contributor can run them rather than merely watch them skip. The would-catch-a-rename proof runs in CI with no checkout at all. Resolution helper shared with **C-46** (S7) in `tests/conftest.py` — the second incident, which is this repo's named trigger for extracting. -**Residual:** the gated half does not run in CI, which needs a views-appwrite checkout in the workflow — a CI-cost and cross-repo-coupling decision, not a code fix. Same shape as **C-46**'s residual and worth deciding once for both (S7 / #188). +**Residual — now tracked as C-81.** The gated half does not run in CI, which needs a views-appwrite checkout in the workflow. That was recorded here and in **C-46** as *"a CI-cost and cross-repo-coupling decision, not a code fix … worth deciding once for both"*, and it sat as a residual on two RESOLVED entries, which is where residuals go to be forgotten. It now has a live entry with a measured cost (17 tests, 9 of them new in this arc), a per-sibling answer, and an owner: **C-81**. views-appwrite is private, so it needs a token — an operator decision. A second, smaller instance of the same shape: these checks parse TOML with `tomllib`, stdlib from Python 3.11, and `pyproject` declares `>=3.11`. CI runs 3.11 and executes them. The maintainer's box runs **3.10**, below the declared floor, so they skip there — the local suite is quietly weaker than a green `pytest -q` suggests. Not a repo defect and not worth its own entry; recorded because "a gate that does not run" is exactly what C-46 is open for, and the CI decision should cover both. | | Tier | 3 | diff --git a/tests/test_doc_accuracy.py b/tests/test_doc_accuracy.py index 4a29779..3e295d0 100644 --- a/tests/test_doc_accuracy.py +++ b/tests/test_doc_accuracy.py @@ -18,6 +18,7 @@ import ast import re +import subprocess from pathlib import Path import pytest @@ -118,6 +119,32 @@ def _link_checked_docs() -> list[Path]: return out + +def test_the_readme_geography_table_matches_the_declared_contract(): + """README's 9-column table is a consumer's implementation spec — check it. + + The order is normative (ADR-013 §5.1) and byte-pinned by the §10 golden fixture, so + a reader built from a reordered table reads the wrong column. Verified 2026-08-03 + that the table had `admin1_gaul0_*` before `admin1_gaul1_*` and typed the four + `*_code` columns `int` where the wire dtype is `float64` — both wrong for months, + because nothing compared the prose to `gaul_schema.COLUMNS`. + """ + from views_postprocessing.contract.gaul_schema import COLUMNS + + rows = re.findall(r"^\|\s*`([a-z0-9_]+)`\s*\|\s*([a-z0-9]+)\s*\|", + (_REPO / "README.md").read_text(), re.M) + documented = [(name, dtype) for name, dtype in rows + if name in {c[0] for c in COLUMNS}] + declared = [(name, dtype) for name, _, dtype in COLUMNS] + + assert documented == declared, ( + f"README's geography table does not match contract/gaul_schema.COLUMNS.\n" + f" README: {documented}\n" + f" declared: {declared}\n" + "The order is normative and the dtypes are the wire's. A consumer builds a " + "reader from this table." + ) + def test_internal_doc_links_resolve(): dead = [] for doc in _link_checked_docs(): @@ -548,20 +575,36 @@ def test_the_procedure_names_who_notifies_and_still_flags_what_fao_has_not_answe ) -def test_the_procedure_distinguishes_intended_policy_from_what_is_implemented(): - """The gap that would otherwise be discovered mid-incident. +def test_the_procedure_names_the_mechanism_that_withdraws_a_bad_run(): + """The first move in an incident must be findable in the first minute. - Withdrawal is the decision; supersession is what the wire actually does, and it is - in force only because nothing else exists. An operator reading this at 22:00 must - not believe a bad delivery becomes unretrievable when it does not. + **This guard used to pin the opposite claim, and pinned it faithfully.** It asserted + the procedure said withdrawal was unbuilt and would cost "an ADR-013 amendment" — + mutation-proven, green for a day, and wrong: views-faoapi had already shipped an + operator quarantine (`f1a59bf`, on its `main`) that drops a manifest file-id from + selection and atomically falls the consumer back to the previous run. The runbook + sent an operator to republish an entire corrected run, for hours, while the wrong + data stayed live. + + ADR-014 §2 asks whether a guard bites. It does not ask whether the thing it bites on + is true — and a guard can hold a falsehood in place perfectly. What this now pins is + the mechanism itself, which is checkable against another repository rather than + against our own prose. """ text = _CORRECTION.read_text() - assert "intended policy is WITHDRAWAL" in text.replace("**", "") - assert "implemented is SUPERSESSION" in text.replace("**", "") - assert "ADR-013 amendment" in text, ( - "the procedure must say what withdrawal would COST — otherwise the gap reads " - "as an oversight rather than as unbuilt work with a known price" + assert "APPWRITE_UNFAO_QUARANTINED_FILE_IDS" in text, ( + "the procedure must name the environment variable that withdraws a run. It is " + "the only step that stops the bleeding, and an operator cannot grep for a " + "mechanism the document does not name." ) + assert "quarantine" in text.lower() + lowered = text.lower() + for token in ("withdrawal", "supersession"): + assert token in lowered, ( + f"the procedure must still distinguish {token} from its sibling — they " + "compose (quarantine withdraws now, publishing supersedes durably) and an " + "operator who conflates them will do only half the correction" + ) def test_no_partner_contact_details_are_published_in_this_repository(): @@ -573,7 +616,33 @@ def test_no_partner_contact_details_are_published_in_this_repository(): as a side effect of documenting a runbook. """ offenders = [] - for doc in (*sorted(_REPO.rglob("*.md")), *sorted(_PKG.rglob("*.py"))): + # This repository's OWN tracked files. `rglob` from the repo root also walks any + # sibling checkout CI places inside the workspace (`_siblings/`) — proven 2026-08-03 + # by planting an address in a checked-out views-crafdapi and watching this fail with + # "appear in this public repository", naming a file in a different repository. A + # commit in someone else's repo could turn `main` red here. + # + # The pathspec is the DIRECTORY, with the suffix filtered in Python. Not + # `views_postprocessing/**/*.py`: git's `**` does not match a top-level file, so that + # form silently skips anything added directly to `views_postprocessing/` — verified + # 2026-08-03 with a probe file. A security-adjacent scan on a public repository must + # not narrow by accident. + tracked = subprocess.run( + ["git", "-C", str(_REPO), "ls-files", "-z", "*.md", "views_postprocessing"], + capture_output=True, text=True, check=False, timeout=30, + ).stdout.split("\0") + scanned = [ + _REPO / name for name in tracked + if name and name.endswith((".md", ".py")) and (_REPO / name).exists() + ] + assert scanned, ( + "git ls-files returned nothing — this scan would pass over an empty set and " + "report success, which is register C-74's exact shape. If this repository is " + "not a git checkout, the guard cannot run and must say so rather than pass." + ) + for doc in sorted(scanned): + if doc.resolve() == Path(__file__).resolve(): + continue # this file names the pattern in order to ban it if ".git" in doc.parts: continue for number, line in enumerate(doc.read_text(errors="ignore").splitlines(), 1): diff --git a/tests/test_env_declaration.py b/tests/test_env_declaration.py index ad34e8e..8be6bd1 100644 --- a/tests/test_env_declaration.py +++ b/tests/test_env_declaration.py @@ -26,6 +26,7 @@ import ast import logging import re +import subprocess from pathlib import Path import pytest @@ -42,7 +43,8 @@ from views_postprocessing.crafd import appwrite_env as crafd_env from views_postprocessing.unfao import appwrite_env -_PKG = Path(__file__).resolve().parent.parent / "views_postprocessing" +_REPO = Path(__file__).resolve().parent.parent +_PKG = _REPO / "views_postprocessing" def _manager_source(partner: str) -> Path: @@ -110,6 +112,10 @@ def _manager_source(partner: str) -> Path: _PARTNERS = tuple(_PARTNER_ENV) +#: Every coordinate name any partner declares — the left-hand sides a document could +#: assign a registry value to. +_EXPECTED_NAMES = {name for _, _, expected in _PARTNER_ENV.values() for name in expected} + #: Function names that belong to python-dotenv and to essentially nothing else. #: @@ -603,8 +609,55 @@ def test_no_coordinate_value_is_copied_into_this_repo(): and id(node) not in docstrings ): copied.append(f"{source.relative_to(_PKG)}:{node.lineno} = {node.value!r}") + # Markdown too — the AST half cannot see a fenced ``bash`` block, and that is exactly + # where four production-forecasts values sat: in README.md's Configuration section, + # two lines below the sentence promising they are never copied, in a PUBLIC + # repository. A guard scoped to `.py` while the rule is about the repository is + # register C-74's shape (2026-08-03). + # + # **What counts as a copy, and what does not.** A first draft flagged any line + # containing a registry value and immediately fired on a dozen documents that merely + # *name* a store in prose — "six stranded documents in unfao_bucket". That is not a + # copy; it is a sentence. C-57 recorded the identical false-positive class over `.py` + # and the identical lesson: when a guard cries wolf, the matching is wrong before the + # scope is (ADR-014 §3). + # + # The copy is a value **assigned to its own coordinate name** — `APPWRITE_X=value` — + # which is a reader's instruction to configure with that literal. That is precise + # enough to have caught README.md and to ignore every legitimate mention. + assignment = re.compile( + r"^\s*(?:export\s+)?(" + "|".join(sorted(_EXPECTED_NAMES)) + r")\s*=\s*(.+?)\s*$" + ) + # This repository's OWN tracked markdown — `git ls-files`, not `rglob`. CI checks + # sibling repositories out into the workspace, and their documents are not this + # repo's to police; an rglob would scan them and fail on someone else's prose. + # + # **This half cannot run in CI**, and that is worth stating rather than discovering. + # The registry values come from the views-appwrite checkout, which is private and + # deliberately not checked out — so the whole test skips there. It guards a + # maintainer's commit, not the merge. Closing that is C-81's token decision. + tracked = subprocess.run( + ["git", "-C", str(_REPO), "ls-files", "-z", "*.md"], + capture_output=True, text=True, check=False, timeout=30, + ).stdout.split("\0") + scanned = [_REPO / name for name in tracked if name and (_REPO / name).exists()] + assert scanned, ( + "git ls-files returned no markdown — the scan would pass over an empty set and " + "report success (register C-74's shape). If this is not a git checkout, the " + "guard cannot run and must say so rather than pass." + ) + for doc in sorted(scanned): + for number, line in enumerate(doc.read_text().splitlines(), 1): + match = assignment.match(line) + if match and match.group(2).strip('"\'') in values: + copied.append( + f"{doc.relative_to(_REPO)}:{number} = {match.group(2)!r} " + f"(assigned to {match.group(1)})" + ) + assert not copied, ( - f"coordinate value(s) from the registry are copied into code: {copied}. The " + f"coordinate value(s) from the registry are copied into this repo: {copied}. The " "registry is referenced, never copied — values reach this package through the " - "environment the launcher assembles, validated by assert_env_declared." + "environment the launcher assembles, validated by assert_env_declared. In a " + "document, write the NAME and leave the value to the launcher." ) diff --git a/views_postprocessing/unfao/managers/README.md b/views_postprocessing/unfao/managers/README.md index b3aa405..701f8f1 100644 --- a/views_postprocessing/unfao/managers/README.md +++ b/views_postprocessing/unfao/managers/README.md @@ -91,5 +91,5 @@ Appwrite). - [`role_and_seams.md`](../../../docs/architecture/role_and_seams.md) — role + seams - [CIC: UNFAOPostProcessorManager](../../../docs/CICs/UNFAOPostProcessorManager.md) — class contract -- [CIC: GaulLookupEnricher](../../../docs/CICs/GaulLookupEnricher.md) — the enrichment join +- [CIC: GaulLookupEnricher](../../../docs/CICs/GaulLookupEnricher.md) — the build/verification path for the GAUL lookup. **Not on this manager's call graph** (register C-75); geography is attached by `contract/historical.py` and `contract/wire/sidecar.py` - ADR-011 (mapper → lookup), ADR-012 (current ontology)