diff --git a/README.md b/README.md index 4f4cec9..361fbd4 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ Requires **Python 3.11–3.14**. | Package | Version | Why | |---------|---------|-----| | `views-pipeline-core` | `>=3.0.0,<4.0.0` (with the `appwrite` extra) | The framework: lifecycle base classes, data loader, dataset container, Appwrite/datastore tools | -| `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) | +| `views-frames` | `>=1.10.2,<2` | The frame data contract — **the live delivery representation** since #126. There is no pandas in this package at all since #90 | | `pyarrow` | `>=16.1.0,<17.0.0` | The wire's serialisation. **Pinned deliberately** — the CVE fix past 17 changes delivered bytes (register C-72) | | *dev group* | `pytest`, `ruff` | Not installed by `pip install views-postprocessing`; `poetry install` includes them | @@ -142,7 +142,6 @@ views-postprocessing/ │ ├── historical.py # the historical artifact, built pandas-free │ ├── gaul_lookup.py # the GAUL asset: path, version, one read │ ├── gaul_schema.py # the 9-column contract, declared as data - │ ├── enrichment.py # GaulLookupEnricher (build/verification path) │ ├── source_metadata.py # producer (datafactory) facts │ ├── store_metadata.py # prediction-store facts │ └── launch_config.py # the delivery mode the launcher must declare @@ -220,7 +219,7 @@ only `.py`; it now scans markdown too.)* |-----|----------------| | [`docs/architecture/role_and_seams.md`](docs/architecture/role_and_seams.md) | **Start here** — role vs the sibling repos + internal seams | | [`docs/ADRs/`](docs/ADRs/) | Architecture decisions (esp. ADR-011 mapper→lookup; ADR-012 ontology) | -| [`docs/CICs/`](docs/CICs/) | Class intent contracts (`UNFAOPostProcessorManager`, `GaulLookupEnricher`) | +| [`docs/CICs/`](docs/CICs/) | Class intent contracts (`UNFAOPostProcessorManager`) | | `reports/technical_risk_register.md` | Tracked risks — C-40 (the remaining pipeline-core inheritance), C-30/C-15 (delivery guards), C-43 (enrichment value verification) | --- diff --git a/docs/ADRs/012_revised_ontology.md b/docs/ADRs/012_revised_ontology.md index 433e70a..9bc11d4 100644 --- a/docs/ADRs/012_revised_ontology.md +++ b/docs/ADRs/012_revised_ontology.md @@ -14,7 +14,7 @@ a **runtime spatial-mapping engine** and **bundled shapefiles** as the authorita stable core. That architecture no longer exists: - [ADR-011](011_replace_runtime_mapper_with_precomputed_lookup.md) replaced the runtime - geopandas mapper with a **precomputed GAUL lookup table** (`GaulLookupEnricher`). + geopandas mapper with a **precomputed GAUL lookup table**. - The mapper, shapefiles, and caching machinery were deleted (C-39 / PR #42). - Input-integrity invariants and structured delivery were added (epic #51). @@ -60,7 +60,7 @@ described the cut-a-repo case; it now says which is which. | **Delivery Invariants** | Representation-free rules over primitives that a delivery must satisfy: coverage, no-collapse, gid parity, observed-range, provenance. Live in `delivery/` — **nothing there imports pandas or views_frames**. *Forecast identity was one of these until 2026-07-31 — see the amendment below.* | Authoritative — they define what a valid delivery is | Stable — changes are governance decisions | | **Representation Seam** | `contract/frame_extraction.py` — turns a `views_frames` frame into the primitives the invariants consume. **One seam.** Its pandas sibling `unfao/extraction.py` was deleted in #151 once the pandas delivery was retired; the two ran as deliberate WET siblings through the migration. | Derived — isolates the representation so invariants stay representation-free | Evolving | | **Wire Mechanism** | `contract/wire/` — the ADR-013 contract: header, shard, sidecar, run manifest, sink, source selection. Partner-neutral: it takes its consumer name and collapse floor as **arguments** (#153). | Authoritative — the contract with the consumer | Stable — changes are contract amendments | -| **Enrichment Asset** | The precomputed GAUL lookup (`data/gaul_lookup.parquet`), its identity in `contract/gaul_lookup.py`, its schema in `contract/gaul_schema.py`, and the keyed gather that joins it (`contract/enrichment.py`, the build/verification path — numpy/pyarrow since #89; see register **C-75** on whether that class should survive at all). | Authoritative for geographic metadata | Stable — rebuilt only when the producer releases new GAUL data | +| **Enrichment Asset** | The precomputed GAUL lookup (`data/gaul_lookup.parquet`), its identity in `contract/gaul_lookup.py`, its schema in `contract/gaul_schema.py`, and the keyed gathers that join it — `contract/historical.py` for the actuals artifact and `contract/wire/sidecar.py` for the §5 sidecar. *(A second gather lived in `contract/enrichment.py` with no production caller; retired in #90, register **C-75**.)* | Authoritative for geographic metadata | Stable — rebuilt only when the producer releases new GAUL data | | **Artifact Builders** | `contract/historical.py` — turns a frame plus the lookup into the partner-facing artifact. | Derived | Evolving | | **External Facts** | Facts read from systems this repo does not own: the producer's (`contract/source_metadata.py` — `last_valid_month_id`, D-07) and the store's (`contract/store_metadata.py`). | Authoritative (the owning system is the source of truth) | Evolving | | **Launch Declarations** | `contract/launch_config.py` — the delivery mode the launcher must declare. Omitting a key is **refused by name**, never inferred (ADR-003, register C-63). | Authoritative | Stable | @@ -73,7 +73,7 @@ described the cut-a-repo case; it now says which is which. 636 lines holding two of everything, and it called `unfao/extraction.py` *"the **single** pandas-aware module"* when pandas lived in three. Both drifted the same way: the ADR described the intended end state of a migration that then stopped one step short. Both are now true — -pandas has **zero runtime importers** (#89 made `contract/enrichment.py`'s a type-only import under `if TYPE_CHECKING` — pandas is in that class's interface, not its implementation) — and the load-bearing ones are +pandas is **absent from the package entirely** (#89 reduced the last one to a type-only import; #90 retired the module that held it — register C-75) — and the load-bearing ones are **mechanically checked** by `tests/test_doc_accuracy.py`, so the next drift fails CI instead of waiting for an audit. diff --git a/docs/CICs/GaulLookupEnricher.md b/docs/CICs/GaulLookupEnricher.md deleted file mode 100644 index a49cfc5..0000000 --- a/docs/CICs/GaulLookupEnricher.md +++ /dev/null @@ -1,193 +0,0 @@ -# Class Intent Contract: GaulLookupEnricher - -**Status:** Draft -**Owner:** PRIO MD&D Team -**Last reviewed:** 2026-06-18 -**Related ADRs:** ADR-011 (replace runtime mapper with precomputed lookup) - ---- - -## 1. Purpose - -> Attach the 9 geographic metadata columns to a prediction frame by merging a -> precomputed GAUL lookup table on the PRIO-GRID cell id. - -It is the lookup-based replacement for `PriogridCountryMapper`'s runtime spatial -enrichment: the spatial computation has already happened upstream (the -views-datafactory area-majority join), so this class does only a table join. - ---- - -## 2. Non-Goals (Explicit Exclusions) - -- This class does **not** perform spatial computation — no geometry, no - shapefiles, no geopandas, no area-majority calculation. -- This class does **not** build the lookup table (that is - `scripts/build_gaul_lookup.py`, run offline). -- This class does **not** fill, impute, or invent metadata for unmatched cells. -- This class does **not** validate the result. Null/coverage enforcement lives on the - delivery path — `contract/historical.assert_metadata_complete` at artifact build and - `delivery/coverage.py` for the region contract. It is **not** the manager's - `_validate()`, which stopped null-gating in #149 and now asserts only that the read - resolved. (Corrected 2026-08-02; PR #200 retired the same claim in the manager's CIC - and this one was left standing.) -- This class does **not** read from the datafactory, viewser, or Appwrite. - ---- - -## 3. Responsibilities and Guarantees - -- Loads exactly one lookup Parquet at construction and verifies it carries the key - plus the 9 contract columns; missing columns raise at construction, and so does an - **empty** lookup — every cell would gather to null and the delivery would then - complain about missing metadata rather than about a missing lookup (S4 / #89). -- Returns the input frame augmented with exactly the 9 columns of - `gaul_schema.METADATA_COLS`: codes numeric, coordinates float, **names and iso as - `object`**. - *Changed in S4 (#89).* They were `category`, inherited from the pandas merge that - read the artifact's dictionary encoding. The gather that replaced it assigns plain - values. Measured on a 200-row output: category 1,795,191 bytes, object 60,061 — - a categorical carries the artifact's full 64,742-entry dictionary whatever the - output size. The builder still writes the artifact with dictionary-encoded names - (`# names/iso categorical (C-32 memory)`); that governs the file, not this output. -- A cell id present in the lookup is enriched with that cell's metadata. -- A cell id **absent** from the lookup yields **null** metadata for that row — - never a sentinel, never a fabricated value (fail-loud downstream). -- Row count, row order **and index** of the input are preserved. Pinned by - `tests/test_enrichment.py::TestFramePropertiesPreserved` across six shapes — - ordered, reversed, duplicated, single, empty, and a non-default index. - (An earlier draft of this line claimed *eight* shapes "including unknown gids", - counting a throwaway development script rather than the committed suite, and naming - a shape that class does not exercise. Unknown gids are covered, for null-value - correctness, by `TestFailLoud` — a different guarantee.) - On **empty** input this is now *more* true than before: the pandas merge replaced the - input's `RangeIndex` with an object-dtype `Index`, where the gather leaves it - untouched. The only behavioural difference found, and it is in the direction the - guarantee above already claimed. - ---- - -## 4. Inputs and Assumptions - -- A lookup Parquet exists at the configured path (default: the committed - `views_postprocessing/data/gaul_lookup.parquet`), indexed by `priogrid_gid`, - containing only fully-complete cells (no nulls, no `-1`, no empty strings). -- The input DataFrame has a column named by `pg_id_col` holding PRIO-GRID cell - ids; a missing `pg_id_col` raises `ValueError`. -- The lookup is the single source of geographic truth — the caller does not - expect this class to reconcile it against any other source. - ---- - -## 5. Outputs and Side Effects - -- Output: the input frame (or, with `only_metadata=True`, just `pg_id_col` + - `time_id_col`) with the 9 metadata columns attached by **keyed gather**. - *Changed in S4 (#89):* this was a pandas left-merge on the lookup's index. The - lookup is now read with pyarrow, sorted once, and addressed by `np.searchsorted`. - Attaching metadata to a frame is a keyed gather, not frame algebra, and it never - needed a merge — which is also what frees the builder to stop writing pandas index - metadata (S5 / #90). -- Public attribute: `lookup_version` — a short, stampable id read from the - lookup's **declared** `lookup_version` metadata key at construction - (`@<8-char source digest>`). Delegates to - `contract.gaul_lookup.version`. The manager stamps it on each delivery so a - delivery is traceable to the exact lookup build. - **It does not degrade.** An artifact carrying no declared key raises - `gaul_lookup.LookupVersionError` (logged at ERROR first, per ADR-008) rather - than returning a placeholder. Until S5 (#186) this returned the string - `"unknown"` whenever views-datafactory's ingestion-ledger shape moved under - it — silently, in the one field register C-15 exists to answer *after* a - suspect delivery. See register **C-60**. -- Side effects: logs the lookup size + version at construction (INFO); logs a - WARNING with the count and sample of unmatched cell ids when any occur; logs - ignored mapper-only kwargs at DEBUG. No file writes, no network. - ---- - -## 6. Failure Modes and Loudness - -- **Raises** at construction if the lookup file is missing or lacks a contract - column. -- **Raises** `ValueError` if `pg_id_col` is not in the input. -- **Does not raise** on unmatched cells — it surfaces them as nulls and logs a - WARNING naming the unknown cells, and separately counting rows that carried no - usable cell id at all. This is deliberate: enforcement is a single point on the - delivery path (`historical.assert_metadata_complete`), so a coverage hole fails - loudly there, not in two places. Passing a sentinel for unmatched cells would be a **bug** (it would - bypass that gate). Aligns with ADR-003 (fail loud on semantic ambiguity). - ---- - -## 7. Boundaries and Interactions - -- Allowed to depend on: numpy, pyarrow, `gaul_schema`, and a local Parquet file. - **pandas is interface-only** — callers hand this class DataFrames and get one back, - but nothing here constructs, reads or joins one, and its import is under - `if TYPE_CHECKING` (S4 / #89). `tests/test_doc_accuracy.py` asserts by AST that the - package has **zero** runtime pandas importers. -- Must **not** depend on: geopandas/shapely, the runtime mapper, the - datafactory, viewser, Appwrite, or any network resource. -- Treats the lookup table as an opaque, trusted artifact produced by the build - script; it does not re-validate the table's spatial correctness. - ---- - -## 8. Examples of Correct Usage - -```python -enricher = GaulLookupEnricher() -out = enricher.enrich_dataframe_with_pg_info( - df.reset_index(), pg_id_col="priogrid_gid", time_id_col="month_id", - only_metadata=True, -) -# out has the 9 metadata columns; unmatched cells are null. -``` - -Signature-compatible with the runtime mapper it replaced (same method name and key -kwargs), which is why the mapper-only kwargs are still accepted and ignored. -**The manager does not call this class** — it has no `enrich` reference at all, and -reads the lookup directly via `gaul_lookup.load()` (register C-66). This example is a -build/verification-path usage. Whether the class should survive that is register -**C-75**. - ---- - -## 9. Examples of Incorrect Usage - -- Filling unmatched cells with `-1`/`""`/`"unknown"` to "avoid validation - errors" — defeats the fail-loud contract and ships wrong data to FAO. -- Using it to enrich against a lookup built for a different region without - expecting nulls for out-of-region cells. -- Calling it expecting spatial recomputation when the lookup is stale — regenerate - the lookup with the build script instead. - ---- - -## 10. Test Alignment - -`tests/test_enrichment.py`: -- **Green:** the 9 columns present; values match the lookup; row count preserved; - codes numeric / coords float. -- **Beige:** lookup integrity (cell count, no nulls, no `-1`, dtypes); coordinate - formula (independent oracle). -- **Red:** unknown cell id and excluded ocean cells yield null; missing - `pg_id_col` raises. - ---- - -## 11. Evolution Notes - -- Stable: the 9-column contract and the fail-loud-on-unmatched behavior (changing - either is a coordinated change across this repo and views-faoapi). -- Expected to change: the lookup's cell-set/region and its source GAUL version, - via re-running the build script. Such regenerations must not change the schema. - ---- - -## End of Contract - -This document defines the **intended meaning** of `GaulLookupEnricher`. - -Changes to behavior that violate this intent are bugs. -Changes to intent must update this contract. diff --git a/docs/CICs/README.md b/docs/CICs/README.md index 177b1d6..4b2b88b 100644 --- a/docs/CICs/README.md +++ b/docs/CICs/README.md @@ -51,7 +51,6 @@ Contracts must be clear enough that: ## Active Contracts - `UNFAOPostProcessorManager.md` — Pipeline orchestration manager (read → transform → validate → save) -- `GaulLookupEnricher.md` — Precomputed GAUL lookup enrichment (ADR-011; replaced the runtime mapper) --- diff --git a/docs/architecture/role_and_seams.md b/docs/architecture/role_and_seams.md index 1a4e99a..9af8da0 100644 --- a/docs/architecture/role_and_seams.md +++ b/docs/architecture/role_and_seams.md @@ -136,7 +136,7 @@ historical actuals arrive as a `FeatureFrame` via pipeline-core's `get_feature_f (#126 — this repo was its first production consumer), the forecast interior is `PredictionFrame`, and `contract/frames.py` is the **live constructor** every interior frame is built through — not the unused adapter it was when this was written, and it supports -`S > 1`. pandas survives in exactly one module, `contract/enrichment.py`, on the +`S > 1`. pandas is gone from the package entirely since #90 retired the last module holding it (register C-75); what remains is on the build/verification path. What **remains** of **register C-40** is narrower than this paragraph implies: the manager is @@ -181,7 +181,6 @@ views_postprocessing/ │ ├── historical.py the historical artifact, built pandas-free │ ├── gaul_lookup.py the GAUL asset: path, version, one read per delivery │ ├── gaul_schema.py the 9-column contract, declared as data -│ ├── enrichment.py GaulLookupEnricher (build/verification path; the last pandas) │ ├── source_metadata.py producer (datafactory) facts, e.g. last_valid_month_id │ ├── store_metadata.py prediction-store facts │ └── launch_config.py the delivery mode the launcher must declare @@ -204,7 +203,7 @@ views_postprocessing/ ## 7. Where to go next - **What was decided and why** → `docs/ADRs/` (esp. ADR-011 mapper→lookup; ADR-012 ontology). -- **Per-class contracts** → `docs/CICs/` (`UNFAOPostProcessorManager`, `GaulLookupEnricher`). +- **Per-class contracts** → `docs/CICs/` (`UNFAOPostProcessorManager`). - **Live risks / open constraints** → the technical risk register (C-40 the pandas gate, C-25/C-30/C-15 the delivery guards). - **The frame/draws future** → #45 (delivery-side draw carrier) and C-40. diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 99292bf..548f439 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -6,8 +6,8 @@ | Owner | Dylan Pinheiro / PRIO MD&D Team | | Last Updated | 2026-08-03 | | Total Concerns | 83 | -| Open Concerns | 15 | -| Resolved Concerns | 68 | +| Open Concerns | 14 | +| Resolved Concerns | 69 | --- @@ -421,41 +421,6 @@ Cross-refs: **C-62** (the transitive dependency drag; the other 31 alerts), **C- --- -### C-75: `GaulLookupEnricher` has no production caller, and now implements a second copy of the delivery path's keyed gather - -| Field | Value | -|-------|-------| -| ID | C-75 | -| Tier | 3 — no correctness impact today: the class is off the delivery path, so a defect in it cannot reach the UN FAO. The cost is that **the verification path and the delivery path now implement the same algorithm twice**, and the tests that check the artifact run through the copy that does *not* ship. A fix applied to one and not the other makes the verification stop verifying what ships — quietly, because both would still pass their own tests. | -| Source | `code-review max` (2026-08-02) — PR #210, five parallel reviewers; two reached this independently | -| Trigger | When a bug is fixed in `contract/historical.py`'s gather (the one that ships), check whether `contract/enrichment.py`'s copy needs the same fix — nothing links them. Also fires at **S5 (#90)**: once the builder is pyarrow-native, the enricher's pandas interface is the last one in the package, and the question "does this class survive?" has to be answered rather than deferred again. | -| Owner | Whoever takes **#90** — the keep-or-retire decision is theirs to make and record, not to defer a third time. Added 2026-08-03: the first draft of this entry named two triggers and no owner, while citing ADR-014 §4 in its own body. This register had already learned that twice — *"a deferral needs an owner and a trigger, not just a reason"* (Cluster L) and *"a decision awaiting an owner, not a task awaiting effort"* (epic #181 closeout). | -| Location | `views_postprocessing/contract/enrichment.py` (the whole class; `_gather` specifically); the shipping twin is `views_postprocessing/contract/historical.py:54-68` | - -**Verified, not inferred (2026-08-02):** `grep -rn "GaulLookupEnricher\|enrich_dataframe_with_pg_info"` across the package finds **zero** production callers — the two hits are docstring mentions in `gaul_lookup.py`. The manager calls `gaul_lookup.load()` directly and has zero `enrich` references. **C-66**'s resolution already said this plainly: *"the pandas enricher leaves the delivery path entirely."* - -**What PR #210 did, and why that raises the question.** S4 (#89) rewrote this class's lookup side from a pandas merge to a numpy/pyarrow keyed gather: a measured dtype analysis, an empty-lookup guard, a mutation-proven bug fix, a corrected CIC, and five reviewers' attention. All of it spent on a method with no reachable caller outside its own test suite. The engineering is sound; what is missing is anyone having **decided** that the class should exist. - -**The duplication is the concrete consequence.** `_gather`'s `argsort → searchsorted → clip → equality-mask` is the same shape as `historical.py:54-68`. The policies differ deliberately — `historical` **raises** on an absent gid (*"geography must never silently vanish"*), the enricher returns nulls for the downstream gate to catch — so extracting a shared helper would mean parameterising the failure policy, which is the guessed abstraction **WET before DRY** exists to prevent. Two copies that are understood is the right call *today*. The trigger above is what stops "today" lasting indefinitely, per **ADR-014 §4**. - -**The precedent is C-45**, `unfao/frames.py`: an unused adapter carried on no live path, resolved by deleting it. This is the same shape with a different module, and the same question — keep it as the declared verification/reference implementation, or retire it and let the fidelity suite test `historical.py` directly. - -**DECISION 2026-08-04 (#90), which this entry's Owner field required of whoever took it: RETIRE.** - -The conditions are no longer arguable. The class has **zero production callers** — only three test files import it. Its last stated justification was "the build/verification path", and #90 rewrote that path arrow-native without touching it, so the justification is spent. It holds the package's **last pandas reference** (a `TYPE_CHECKING` import), which is the one thing standing between epic #85 and an honest close. And its `_gather` duplicates `contract/historical.py`'s shipping gather, which is independently covered by four test files. - -**C-45 is the precedent and it was resolved by deleting.** Same shape, different module. - -**Not executed in #90, deliberately.** The retirement touches ten files — the module, its 39 tests, references in two other test files, its CIC, ADR-012, `gaul_lookup.py`'s docstring, the machinery list in `test_clone_readiness.py`, and the pandas-importer assertion in `test_doc_accuracy.py`. Folding that into a builder rewrite would mix a behaviour-preserving change with a large deletion, which is the thing epic #148's S5 explicitly refused to do. It is the next change, not a later one. - -**Epic #85 and tracking #93 stay open until it lands**, because their claim — pandas pushed to the seams — only becomes true when this module is gone. - -**Deliberately NOT registered from the same review** (defects in unmerged code, all fixed in #210 before merge rather than tracked): a NaN gid crashing the warning path, the unvalidated int64 coercion at both ends, the AST guard's `else`-branch blind spot, ADR-012's stale pandas-merge claim, and three CIC claims retired elsewhere by #200. The register tracks standing risk; a defect fixed before it ships is not one. They are recorded in the PR. - -Cross-refs: **C-45** (RESOLVED — the same shape, resolved by deletion), **C-66** (RESOLVED — established the enricher left the delivery path), **C-40** (which calls `enrichment.py` and `extraction.py` together *"the retired-in-place `enrichment.py`/`extraction.py` legacy seams"*), **#89** / **#90** / epic **#85**, ADR-014 §4. - ---- - ### C-80: The doc-accuracy scan exempts ADRs and CICs — the two artifact classes that define the contracts | Field | Value | @@ -664,6 +629,51 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-75: `GaulLookupEnricher` has no production caller, and now implements a second copy of the delivery path's keyed gather — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-75 | +| Tier | 3 — no correctness impact today: the class is off the delivery path, so a defect in it cannot reach the UN FAO. The cost is that **the verification path and the delivery path now implement the same algorithm twice**, and the tests that check the artifact run through the copy that does *not* ship. A fix applied to one and not the other makes the verification stop verifying what ships — quietly, because both would still pass their own tests. | +| Source | `code-review max` (2026-08-02) — PR #210, five parallel reviewers; two reached this independently | +| Trigger | When a bug is fixed in `contract/historical.py`'s gather (the one that ships), check whether `contract/enrichment.py`'s copy needs the same fix — nothing links them. Also fires at **S5 (#90)**: once the builder is pyarrow-native, the enricher's pandas interface is the last one in the package, and the question "does this class survive?" has to be answered rather than deferred again. | +| Owner | Whoever takes **#90** — the keep-or-retire decision is theirs to make and record, not to defer a third time. Added 2026-08-03: the first draft of this entry named two triggers and no owner, while citing ADR-014 §4 in its own body. This register had already learned that twice — *"a deferral needs an owner and a trigger, not just a reason"* (Cluster L) and *"a decision awaiting an owner, not a task awaiting effort"* (epic #181 closeout). | +| Location | `views_postprocessing/contract/enrichment.py` (the whole class; `_gather` specifically); the shipping twin is `views_postprocessing/contract/historical.py:54-68` | + +**Verified, not inferred (2026-08-02):** `grep -rn "GaulLookupEnricher\|enrich_dataframe_with_pg_info"` across the package finds **zero** production callers — the two hits are docstring mentions in `gaul_lookup.py`. The manager calls `gaul_lookup.load()` directly and has zero `enrich` references. **C-66**'s resolution already said this plainly: *"the pandas enricher leaves the delivery path entirely."* + +**What PR #210 did, and why that raises the question.** S4 (#89) rewrote this class's lookup side from a pandas merge to a numpy/pyarrow keyed gather: a measured dtype analysis, an empty-lookup guard, a mutation-proven bug fix, a corrected CIC, and five reviewers' attention. All of it spent on a method with no reachable caller outside its own test suite. The engineering is sound; what is missing is anyone having **decided** that the class should exist. + +**The duplication is the concrete consequence.** `_gather`'s `argsort → searchsorted → clip → equality-mask` is the same shape as `historical.py:54-68`. The policies differ deliberately — `historical` **raises** on an absent gid (*"geography must never silently vanish"*), the enricher returns nulls for the downstream gate to catch — so extracting a shared helper would mean parameterising the failure policy, which is the guessed abstraction **WET before DRY** exists to prevent. Two copies that are understood is the right call *today*. The trigger above is what stops "today" lasting indefinitely, per **ADR-014 §4**. + +**The precedent is C-45**, `unfao/frames.py`: an unused adapter carried on no live path, resolved by deleting it. This is the same shape with a different module, and the same question — keep it as the declared verification/reference implementation, or retire it and let the fidelity suite test `historical.py` directly. + +**DECISION 2026-08-04 (#90), which this entry's Owner field required of whoever took it: RETIRE.** + +The conditions are no longer arguable. The class has **zero production callers** — only three test files import it. Its last stated justification was "the build/verification path", and #90 rewrote that path arrow-native without touching it, so the justification is spent. It holds the package's **last pandas reference** (a `TYPE_CHECKING` import), which is the one thing standing between epic #85 and an honest close. And its `_gather` duplicates `contract/historical.py`'s shipping gather, which is independently covered by four test files. + +**C-45 is the precedent and it was resolved by deleting.** Same shape, different module. + +**Not executed in #90, deliberately.** The retirement touches ten files — the module, its 39 tests, references in two other test files, its CIC, ADR-012, `gaul_lookup.py`'s docstring, the machinery list in `test_clone_readiness.py`, and the pandas-importer assertion in `test_doc_accuracy.py`. Folding that into a builder rewrite would mix a behaviour-preserving change with a large deletion, which is the thing epic #148's S5 explicitly refused to do. It is the next change, not a later one. + +**Epic #85 and tracking #93 stay open until it lands**, because their claim — pandas pushed to the seams — only becomes true when this module is gone. + +**Deliberately NOT registered from the same review** (defects in unmerged code, all fixed in #210 before merge rather than tracked): a NaN gid crashing the warning path, the unvalidated int64 coercion at both ends, the AST guard's `else`-branch blind spot, ADR-012's stale pandas-merge claim, and three CIC claims retired elsewhere by #200. The register tracks standing risk; a defect fixed before it ships is not one. They are recorded in the PR. + +Cross-refs: **C-45** (RESOLVED — the same shape, resolved by deletion), **C-66** (RESOLVED — established the enricher left the delivery path), **C-40** (which calls `enrichment.py` and `extraction.py` together *"the retired-in-place `enrichment.py`/`extraction.py` legacy seams"*), **#89** / **#90** / epic **#85**, ADR-014 §4. + +**RESOLVED 2026-08-04 — retired, as the decision recorded above required.** `views_postprocessing/contract/enrichment.py` and its 39 tests are deleted, along with `docs/CICs/GaulLookupEnricher.md`. + +**No coverage of shipping code was lost.** The two tests elsewhere that imported the class both asserted only that its `lookup_version` agreed with `gaul_lookup.version()` — two readers of one fact, checked against each other. They now read the fact through the declared reader the delivery itself uses, which is the half that was ever load-bearing. The gather it duplicated is `contract/historical.py`'s, covered independently by four test files. + +**One guard was deleted rather than kept.** `test_gaul_lookup_access.py` asserted `"GaulLookupEnricher" not in` the manager source. With the class gone that assertion cannot fail, and a test that cannot fail is decoration (ADR-014 §2). What it protected — one lookup read per delivery — is the first assertion in the same function and still bites. + +**Fifteen files, and the sweep is the point.** The module, its tests, its CIC, the CIC index, two test files that imported it, the machinery list, the pandas-importer assertion, `README.md`'s dependency table and package tree, `role_and_seams.md`'s tree and contract list, ADR-012's ontology row and its pandas claim, and two module docstrings. Every one of those was a live claim about a class that no longer exists — which is the argument for C-80: none of the ADR or CIC references would have been caught by any guard. + +**What it makes true.** `grep -rn "^import pandas\|^from pandas" views_postprocessing/ scripts/` now returns nothing at all — not a runtime import, not a type-only one. Epic **#85**'s claim, *pandas pushed to the seams*, is finally literal rather than nearly-true, and #85 and #93 close with this. + +--- + ### C-76: `build_gaul_lookup.py` will write an empty lookup without complaint — RESOLVED | Field | Value | diff --git a/tests/test_clone_readiness.py b/tests/test_clone_readiness.py index 549bb77..79413fe 100644 --- a/tests/test_clone_readiness.py +++ b/tests/test_clone_readiness.py @@ -64,7 +64,6 @@ "views_postprocessing.contract.historical", "views_postprocessing.contract.gaul_lookup", "views_postprocessing.contract.gaul_schema", - "views_postprocessing.contract.enrichment", "views_postprocessing.contract.launch_config", "views_postprocessing.contract.source_metadata", "views_postprocessing.contract.store_metadata", diff --git a/tests/test_doc_accuracy.py b/tests/test_doc_accuracy.py index 3e295d0..1669d72 100644 --- a/tests/test_doc_accuracy.py +++ b/tests/test_doc_accuracy.py @@ -253,9 +253,11 @@ def test_pandas_is_not_imported_at_runtime_anywhere_in_the_package(): "the representation the migration removed. If it is genuinely needed, put it " "behind `if TYPE_CHECKING` or say in ADR-012 why it is not." ) - assert type_only == ["contract/enrichment.py"], ( - f"the type-only pandas imports moved: {type_only}. Not necessarily wrong — but " - "ADR-012 names the seam, so update it rather than letting the claim drift." + assert type_only == [], ( + f"a type-only pandas import reappeared: {type_only}. There is no pandas in this " + "package at all since #90 retired contract/enrichment.py (register C-75) — epic " + "#85's claim is now literal, not nearly-true. If one is genuinely needed, say in " + "ADR-012 why before adding it back." ) @@ -448,7 +450,7 @@ def test_the_ban_covers_the_post_148_deletions_and_spares_the_live_lookalike(): ``_BANNED`` says a deletion PR that does not extend it has not finished. No test can substitute for that; only the habit can. 2. **Too broad** — `FAO_PGMDataset` is a *views-faoapi* class, alive and correctly - cited in `contract/gaul_schema.py` and `contract/enrichment.py`. A careless + cited in `contract/gaul_schema.py`. A careless `PGMDataset` pattern would ban a true statement about another repo's code. """ for deleted in ( diff --git a/tests/test_enrichment.py b/tests/test_enrichment.py deleted file mode 100644 index ca1ea6c..0000000 --- a/tests/test_enrichment.py +++ /dev/null @@ -1,419 +0,0 @@ -"""Tests for the lookup-based enricher (ADR-011, Stage 1). - -Run against the committed gaul_lookup.parquet — no datafactory checkout or -shapefiles required. These tests pin the contract the enricher must satisfy so -that swapping it in for the runtime mapper (Stage 3) is invisible downstream. -""" - -import pandas as pd -import logging - -import numpy as np -import pyarrow as pa -import pyarrow.parquet as pq -import pytest - -from views_postprocessing.contract import gaul_lookup -from views_postprocessing.contract.enrichment import ( - GaulLookupEnricher, - METADATA_COLS, -) - -# The 5 africa_me_legacy ocean cells that have no GAUL assignment — they must -# NOT be in the lookup (they are excluded upstream by the land_gaul region). -OCEAN_CELLS = [62356, 94776, 99027, 107733, 107742] - -CODE_COLS = ["admin1_gaul1_code", "admin1_gaul0_code", "admin2_gaul2_code"] -NAME_COLS = ["admin1_gaul1_name", "admin1_gaul0_name", "admin2_gaul2_name", - "country_iso_a3"] - - -@pytest.fixture(scope="module") -def enricher(): - return GaulLookupEnricher() - - -@pytest.fixture(scope="module") -def lookup(): - """The committed artifact as a pandas frame, built here rather than reached for. - - Until S4 (#89) this was ``enricher._lookup`` — the enricher's own private pandas - frame. It no longer has one: the lookup side is numpy + pyarrow, and pandas is a - type-only import there. These tests are about the **artifact**, not the enricher, - so they load it directly. - - The ``set_index`` is deliberate and forward-looking. ``to_pandas()`` currently - restores ``priogrid_gid`` as the index from the parquet's embedded pandas - metadata — metadata that **S5 (#90) removes** when the builder becomes - pyarrow-native. Handling both shapes means these tests do not have to change - again then. - """ - df = gaul_lookup.load().to_pandas() - if "priogrid_gid" in df.columns: - df = df.set_index("priogrid_gid") - return df - - -class TestLookupIntegrity: - """The committed lookup must be clean by construction.""" - - def test_expected_cell_count(self, lookup): - # land_gaul region: 64,742 fully-complete land cells. - assert len(lookup) == 64_742 - - def test_no_nulls_anywhere(self, lookup): - assert int(lookup.isna().sum().sum()) == 0 - - def test_no_minus_one_sentinel_in_codes(self, lookup): - for c in CODE_COLS: - assert (lookup[c] != -1).all(), f"{c} has -1 sentinel" - - def test_no_empty_strings_in_names(self, lookup): - for c in NAME_COLS: - assert (lookup[c].astype(str).str.len() > 0).all() - - def test_has_exactly_the_nine_contract_columns(self, lookup): - assert list(lookup.columns) == METADATA_COLS - - def test_dtypes(self, lookup): - for c in CODE_COLS: - assert pd.api.types.is_numeric_dtype(lookup[c]), c - for c in ["pg_xcoord", "pg_ycoord"]: - assert pd.api.types.is_float_dtype(lookup[c]), c - for c in NAME_COLS: - assert isinstance(lookup[c].dtype, pd.CategoricalDtype), c - - def test_ocean_cells_excluded(self, lookup): - for gid in OCEAN_CELLS: - assert gid not in lookup.index, f"ocean cell {gid} should be excluded" - - -class TestCoordinateFormula: - """pg_xcoord/pg_ycoord must follow the PRIO-GRID 0.5-degree formula.""" - - def test_known_coordinates(self, lookup): - for gid in lookup.index[:50]: - x = -180.0 + ((gid - 1) % 720) * 0.5 + 0.25 - y = -90.0 + ((gid - 1) // 720) * 0.5 + 0.25 - assert lookup.loc[gid, "pg_xcoord"] == pytest.approx(x) - assert lookup.loc[gid, "pg_ycoord"] == pytest.approx(y) - - -class TestEnricherContract: - """enrich() must reproduce the 9-column contract the manager consumes.""" - - def _frame(self, gids): - return pd.DataFrame({ - "priogrid_gid": gids, - "month_id": [100] * len(gids), - }) - - def test_all_metadata_cols_present(self, enricher, lookup): - gids = lookup.index[:10].tolist() - out = enricher.enrich_dataframe_with_pg_info( - self._frame(gids), pg_id_col="priogrid_gid", time_id_col="month_id", - ) - for c in METADATA_COLS: - assert c in out.columns - - def test_no_nulls_for_known_gids(self, enricher, lookup): - gids = lookup.index[:100].tolist() - out = enricher.enrich_dataframe_with_pg_info( - self._frame(gids), pg_id_col="priogrid_gid", time_id_col="month_id", - ) - for c in METADATA_COLS: - assert out[c].isna().sum() == 0, c - - def test_values_match_lookup(self, enricher, lookup): - gids = lookup.index[:20].tolist() - out = enricher.enrich_dataframe_with_pg_info( - self._frame(gids), pg_id_col="priogrid_gid", time_id_col="month_id", - ).set_index("priogrid_gid") - for gid in gids: - assert out.loc[gid, "country_iso_a3"] == lookup.loc[gid, "country_iso_a3"] - assert out.loc[gid, "admin2_gaul2_code"] == lookup.loc[gid, "admin2_gaul2_code"] - - def test_row_count_preserved(self, enricher, lookup): - gids = lookup.index[:7].tolist() - out = enricher.enrich_dataframe_with_pg_info( - self._frame(gids), pg_id_col="priogrid_gid", time_id_col="month_id", - ) - assert len(out) == 7 - - def test_codes_numeric_coords_float(self, enricher, lookup): - gids = lookup.index[:5].tolist() - out = enricher.enrich_dataframe_with_pg_info( - self._frame(gids), pg_id_col="priogrid_gid", time_id_col="month_id", - ) - for c in CODE_COLS: - assert pd.api.types.is_numeric_dtype(out[c]), c - for c in ["pg_xcoord", "pg_ycoord"]: - assert pd.api.types.is_float_dtype(out[c]), c - - -class TestFailLoud: - """Unknown / excluded cells must surface as NaN, not a sentinel.""" - - def test_unknown_gid_yields_null(self, enricher, lookup): - good = int(lookup.index[0]) - df = pd.DataFrame({"priogrid_gid": [good, 999_999], "month_id": [1, 1]}) - out = enricher.enrich_dataframe_with_pg_info( - df, pg_id_col="priogrid_gid", time_id_col="month_id", - ).set_index("priogrid_gid") - assert out.loc[good, "country_iso_a3"] is not None - assert pd.isna(out.loc[999_999, "country_iso_a3"]) - for c in CODE_COLS: - assert pd.isna(out.loc[999_999, c]) - - def test_ocean_cell_yields_null(self, enricher): - df = pd.DataFrame({"priogrid_gid": OCEAN_CELLS, - "month_id": [1] * len(OCEAN_CELLS)}) - out = enricher.enrich_dataframe_with_pg_info( - df, pg_id_col="priogrid_gid", time_id_col="month_id", - ) - # Every ocean cell is absent from the lookup -> all-null metadata. - assert out["country_iso_a3"].isna().all() - - def test_missing_pg_id_col_raises(self, enricher): - with pytest.raises(ValueError, match="not found"): - enricher.enrich_dataframe_with_pg_info( - pd.DataFrame({"x": [1]}), pg_id_col="priogrid_gid", - ) - - -# ── the guarantees the CIC states, pinned (S4 / #89; ADR-014 §1) ───────────── -# -# The CIC claims row/order/index preservation "verified across eight input shapes" -# and names the output dtypes. That verification was a throwaway script run once -# during development — a guarantee resting on a claim rather than on a check, which -# is precisely what ADR-014 §1 forbids, written by the story after the ADR landed. -# These commit it. - - -class TestConstructionRefusals: - """Degenerate lookups must fail at construction, naming the lookup.""" - - def _write(self, path, gids): - cols = {"priogrid_gid": pa.array(gids, pa.int64())} - for c in METADATA_COLS: - cols[c] = pa.array( - [0.0] * len(gids) if c in ("pg_xcoord", "pg_ycoord") or c.endswith("_code") - else ["x"] * len(gids), - pa.float64() if c in ("pg_xcoord", "pg_ycoord") or c.endswith("_code") else pa.string(), - ) - t = pa.table(cols).replace_schema_metadata({b"lookup_version": b"t@00000000"}) - pq.write_table(t, path) - return path - - def test_an_empty_lookup_is_refused_at_construction(self, tmp_path): - """It used to raise IndexError from inside the gather instead — the guard - `(len(self._keys) > 0) & (...)` read as a guard and was not one, because `&` - evaluates both operands.""" - path = self._write(tmp_path / "empty.parquet", []) - with pytest.raises(ValueError, match="is empty"): - GaulLookupEnricher(path) - - def test_a_null_key_is_refused_at_construction(self, tmp_path): - """A null key would coerce to the same sentinel as an unusable query id, the - two would collide, and the row would be reported FOUND — receiving another - cell's metadata. That is the fabricated value this module forbids (cf. C-35).""" - cols = {"priogrid_gid": pa.array([1, None, 3], pa.int64())} - for c in METADATA_COLS: - numeric = c in ("pg_xcoord", "pg_ycoord") or c.endswith("_code") - cols[c] = pa.array([0.0] * 3 if numeric else ["x"] * 3, - pa.float64() if numeric else pa.string()) - path = tmp_path / "nullkey.parquet" - pq.write_table(pa.table(cols).replace_schema_metadata({b"lookup_version": b"t@0"}), path) - with pytest.raises(ValueError, match="null values"): - GaulLookupEnricher(path) - - -class TestUnusableCellIds: - """A value that is not a cell id gathers to null — never to a guessed cell. - - Each case was verified against the pandas merge this replaced; the behaviour - below is that merge's, not an invention. The drifted-float case is the one that - matters most: a blind `astype(np.int64)` truncates `54220.000000001` to `54220` - and matches a real, *different* cell, silently. - """ - - def _out(self, enricher, col): - return enricher.enrich_dataframe_with_pg_info( - pd.DataFrame({"priogrid_gid": col, "month_id": [1] * len(col)}), - pg_id_col="priogrid_gid", time_id_col="month_id", - ) - - def test_a_missing_gid_yields_null_and_does_not_crash(self, enricher, lookup): - good = lookup.index[:2].tolist() - out = self._out(enricher, [float(good[0]), float("nan"), float(good[1])]) - assert len(out) == 3 - assert out["country_iso_a3"].isna().sum() == 1 - - def test_a_non_integral_gid_never_matches_a_neighbouring_cell(self, enricher, lookup): - gid = int(lookup.index[100]) - out = self._out(enricher, [float(gid) + 1e-9]) - assert out["country_iso_a3"].isna().all(), ( - "a drifted float matched a cell — truncation turned it into a different, " - "real gid and fabricated that cell's geography" - ) - - def test_pandas_na_yields_null_rather_than_a_bare_typeerror(self, enricher, lookup): - good = lookup.index[:2].tolist() - out = self._out(enricher, pd.array([good[0], pd.NA, good[1]], dtype="Int64")) - assert len(out) == 3 and out["country_iso_a3"].isna().sum() == 1 - - def test_an_unusable_id_cannot_match_the_cell_it_was_substituted_with(self, tmp_path): - """The `& usable` term in `_gather`, proven load-bearing (ADR-014 §2). - - It survived removal against the committed artifact: an unusable id converts to - `0`, and `0` is not a real gid, so the key comparison already failed. That made - it look like decoration. It is not — it is the only thing standing between an - unusable id and the row it was substituted onto, and the substitute is a real - key the moment a lookup contains gid `0`. - - Without the mask, the NaN row below silently receives cell 0's geography. That - is the fabricated value this module's docstring forbids, produced from an input - the caller never wrote. - """ - cols = {"priogrid_gid": pa.array([0, 5, 9], pa.int64())} - for c in METADATA_COLS: - numeric = c in ("pg_xcoord", "pg_ycoord") or c.endswith("_code") - cols[c] = pa.array( - [0.0] * 3 if numeric else ["ZERO", "FIVE", "NINE"], - pa.float64() if numeric else pa.string(), - ) - path = tmp_path / "withzero.parquet" - pq.write_table( - pa.table(cols).replace_schema_metadata({b"lookup_version": b"t@00000000"}), path - ) - - out = GaulLookupEnricher(path).enrich_dataframe_with_pg_info( - pd.DataFrame({"priogrid_gid": [5.0, float("nan"), 9.0], "month_id": [1, 1, 1]}), - pg_id_col="priogrid_gid", time_id_col="month_id", - ) - assert out["country_iso_a3"].tolist() == ["FIVE", None, "NINE"], ( - "an unusable id matched the cell it was substituted with — `& usable` is " - "what prevents that, and this is the input that proves it" - ) - - def test_a_string_gid_column_is_refused_rather_than_parsed(self, enricher, lookup): - """A string gid is a declaration error, and the merge this replaced said so. - - `pandas` raised *"You are trying to merge on object and int64 columns. If you - wish to proceed you should use pd.concat"*. An - earlier draft of `_as_cell_ids` used `float(value)`, which happily parsed - `"54220"` and matched — inference where the old path declared (ADR-003). - """ - gid = str(int(lookup.index[0])) - out = self._out(enricher, [gid]) - assert out["country_iso_a3"].isna().all(), ( - f"the string {gid!r} was parsed into a cell id; a string column is a " - "declaration error, not a gid to be guessed at" - ) - - @pytest.mark.parametrize( - "label,array", - [ - # Each case must enter the branch it is testing. An earlier draft built - # them all with dtype=object, which routed every one through the - # element-wise branch — so the int and float bounds it claimed to test - # were never executed, and removing them left the suite green. - ("uint64 max (int branch)", np.array([np.iinfo(np.uint64).max], dtype=np.uint64)), - ("1e30 (float branch)", np.array([1e30], dtype=np.float64)), - ("inf (float branch)", np.array([np.inf], dtype=np.float64)), - ("-inf (float branch)", np.array([-np.inf], dtype=np.float64)), - ("string (object branch)", np.array(["123"], dtype=object)), - ("bool (object branch)", np.array([True], dtype=object)), - ("float exactly 2**63", np.array([2.0**63], dtype=np.float64)), - ("timedelta64", np.array([1], dtype="timedelta64[ns]")), - ], - ) - def test_out_of_range_and_non_numeric_values_are_unusable_not_wrapped( - self, enricher, label, array - ): - """Outside int64 the cast WRAPS rather than raising, and a wrapped id flagged - valid is exactly the fabricated value this conversion exists to prevent. - - `uint64` max becomes `-1`; `1e30` and `float(2**63)` become `INT64_MIN`. Each - passes a naive finite-and-integral test, so the bounds are separate conditions - rather than consequences of the others. - - Asserted through the PUBLIC method. An earlier draft called the private - `_as_cell_ids` and checked its mask — pinning an implementation detail rather - than the guarantee a caller relies on, and skipping `_gather`'s use of that - mask entirely. Every dtype below is reachable through a DataFrame column, so - there was never a reason to reach inside. - """ - out = enricher.enrich_dataframe_with_pg_info( - pd.DataFrame({"priogrid_gid": array, "month_id": [1] * len(array)}), - pg_id_col="priogrid_gid", time_id_col="month_id", - ) - assert out["country_iso_a3"].isna().all(), ( - f"{label} produced a match — an id the caller never wrote was treated as " - "a cell and that cell's geography was fabricated onto the row" - ) - - # AND at the mask, because the public assertion above cannot tell "refused" - # from "accepted but wrapped to an id that happens to be absent" — both give - # null. Reverting the float bound or re-admitting timedelta64 left the public - # assertion green while the value was silently accepted; only this sees it. - # The public check is the caller's guarantee; this one is the mutation proof. - _, usable = GaulLookupEnricher._as_cell_ids(array) - assert not usable.any(), ( - f"{label} was ACCEPTED as a cell id (it only looked refused because the " - f"wrapped id is absent from this lookup — against a lookup that contains " - f"it, this row would receive that cell's geography)" - ) - - def test_the_warning_names_unknown_cells_but_invents_no_id_for_unusable_ones( - self, enricher, lookup, caplog - ): - good = int(lookup.index[0]) - with caplog.at_level(logging.WARNING): - self._out(enricher, [float(good), float("nan"), 999999.0]) - message = " ".join(r.getMessage() for r in caplog.records) - assert "999999" in message, "the genuinely unknown cell must be named" - assert "no usable cell id" in message - assert "[0]" not in message, ( - "an unusable id was reported as cell 0 — a cell nobody asked about" - ) - - -class TestFramePropertiesPreserved: - """Row count, order and index survive the gather. Was CIC prose; now a check.""" - - @pytest.mark.parametrize("shape", ["ordered", "reversed", "duplicated", "single", "empty"]) - def test_row_count_and_order_are_preserved(self, enricher, lookup, shape): - gids = { - "ordered": lookup.index[:50].tolist(), - "reversed": lookup.index[:50].tolist()[::-1], - "duplicated": lookup.index[:5].tolist() * 3, - "single": lookup.index[:1].tolist(), - "empty": [], - }[shape] - out = enricher.enrich_dataframe_with_pg_info( - pd.DataFrame({"priogrid_gid": gids, "month_id": [1] * len(gids)}), - pg_id_col="priogrid_gid", time_id_col="month_id", - ) - assert len(out) == len(gids) - assert out["priogrid_gid"].tolist() == gids - - def test_a_non_default_index_is_preserved(self, enricher, lookup): - gids = lookup.index[:20].tolist() - df = pd.DataFrame({"priogrid_gid": gids, "month_id": [1] * 20}, - index=range(1000, 1020)) - out = enricher.enrich_dataframe_with_pg_info( - df, pg_id_col="priogrid_gid", time_id_col="month_id") - assert out.index.equals(df.index), ( - "the index moved. The pandas merge this replaced mutated it on empty " - "input; the gather must not mutate it on any input." - ) - - def test_name_columns_are_object_not_categorical(self, enricher, lookup): - """The CIC states this dtype. It changed in #89 and was measured, not guessed.""" - out = enricher.enrich_dataframe_with_pg_info( - pd.DataFrame({"priogrid_gid": lookup.index[:5].tolist(), "month_id": [1] * 5}), - pg_id_col="priogrid_gid", time_id_col="month_id", - ) - for c in NAME_COLS: - assert out[c].dtype == object, f"{c} is {out[c].dtype}, CIC says object" diff --git a/tests/test_gaul_lookup_access.py b/tests/test_gaul_lookup_access.py index d0de6c6..91ee742 100644 --- a/tests/test_gaul_lookup_access.py +++ b/tests/test_gaul_lookup_access.py @@ -153,19 +153,25 @@ def test_the_manager_reads_the_lookup_once_and_threads_it(): assert "_DEFAULT_LOOKUP" not in source, ( "the manager must not import the enricher's private path constant (C-68)" ) - assert "GaulLookupEnricher" not in source, ( - "the contract delivery does not use the pandas enricher; instantiating it " - "loads the lookup a third time into a representation nothing reads (C-66)" - ) + # The `GaulLookupEnricher` assertion that stood here was removed with the class in + # #90 (register C-75). A guard against a symbol that no longer exists cannot fail, + # and a test that cannot fail is decoration (ADR-014 §2). What it protected — one + # read per delivery — is the first assertion in this function and still bites. -def test_the_enricher_still_works_and_agrees_on_the_stamp(): - """The enricher is not retired — it remains the build/verification path's object. +def test_the_artifacts_identity_comes_from_one_declared_reader(): + """The stamp has exactly one source, and it is the one the delivery reads. - What changed is that it no longer *owns* the artifact's identity. Its - ``lookup_version`` must still report exactly what ``gaul_lookup.version`` does, or - provenance would differ depending on which path produced it. + **This test used to assert that `GaulLookupEnricher.lookup_version` agreed with + `gaul_lookup.version()`** — two readers of the same fact, checked against each + other. The enricher was retired in #90/C-75 as an object with no production caller, + so the agreement it policed no longer has two sides. What survives is the property + that mattered: the version the delivery stamps into its provenance is read from the + artifact, and resolves. """ - from views_postprocessing.contract.enrichment import GaulLookupEnricher - - assert GaulLookupEnricher().lookup_version == gaul_lookup.version() + version = gaul_lookup.version() + assert version, "the lookup must declare a version; C-60 made this raise rather than degrade" + assert "@" in version, ( + f"lookup_version must be '@', got {version!r} — the delivery " + "carries this verbatim into its provenance record (C-15)." + ) diff --git a/tests/test_gaul_lookup_fidelity.py b/tests/test_gaul_lookup_fidelity.py index 821ddfb..ee023c4 100644 --- a/tests/test_gaul_lookup_fidelity.py +++ b/tests/test_gaul_lookup_fidelity.py @@ -218,10 +218,16 @@ def test_lookup_declares_its_provenance(lookup_meta): def test_lookup_version_stamp_resolves(lookup): - """The stamp the delivery provenance carries (C-15) must not be 'unknown'.""" - from views_postprocessing.contract.enrichment import GaulLookupEnricher + """The stamp the delivery provenance carries (C-15) must not be 'unknown'. - version = GaulLookupEnricher(_LOOKUP).lookup_version + Read through ``gaul_lookup.version`` — the declared reader the delivery itself uses. + It was read through ``GaulLookupEnricher`` until that object was retired (#90, C-75); + the enricher was never on the delivery path, so this now checks the same fact through + the code that ships. + """ + from views_postprocessing.contract import gaul_lookup + + version = gaul_lookup.version(_LOOKUP) assert version != "unknown", ( "lookup_version resolved to 'unknown' — the delivery would ship untraceable " "provenance (C-60). The lookup's embedded source_provenance is absent or reshaped." diff --git a/tests/test_input_integrity_design_contract.py b/tests/test_input_integrity_design_contract.py index b86416e..125fc8a 100644 --- a/tests/test_input_integrity_design_contract.py +++ b/tests/test_input_integrity_design_contract.py @@ -79,9 +79,16 @@ def test_manager_does_not_inherit_forecasting_model_manager(): # housekeeping -------------------------------------------------------------- -def test_enrichment_does_not_call_a_dataframe_a_prediction_frame(): - src = (_PKG / "contract" / "enrichment.py").read_text().lower() - assert "prediction frame" not in src +# `test_enrichment_does_not_call_a_dataframe_a_prediction_frame` was removed with +# `contract/enrichment.py` in #90 (register C-75). It asserted that one module did not +# call a DataFrame a "prediction frame" — vocabulary hygiene in a module that no longer +# exists. +# +# Nothing replaces it, and that is the honest position rather than an oversight: the +# confusion it guarded against was possible only while a pandas object sat next to a +# frame-shaped API. `test_delivery_invariants_are_pandas_free` above, and the +# now-empty type-only-pandas assertion in `test_doc_accuracy.py`, together mean there +# is no DataFrame left in the package to mislabel. def test_no_lingering_mapping_directory(): diff --git a/views_postprocessing/contract/enrichment.py b/views_postprocessing/contract/enrichment.py deleted file mode 100644 index 75baa77..0000000 --- a/views_postprocessing/contract/enrichment.py +++ /dev/null @@ -1,317 +0,0 @@ -"""Lookup-based geographic enrichment (ADR-011). - -Drop-in replacement for the runtime spatial mapper's -``enrich_dataframe_with_pg_info``. Instead of loading 774 MB of shapefiles and -computing spatial intersections at run time, it merges a precomputed lookup -table (built by ``scripts/build_gaul_lookup.py`` from the views-datafactory's -area-majority GAUL parquets) onto the input DataFrame by PRIO-GRID cell id. - -No geopandas, no shapefiles, no spatial computation. The lookup contains only -fully-complete cells; an unknown or incomplete cell id gathers to null, so the -delivery's null gate still crashes (fail-loud) rather than shipping a hole. This -is intentional and matches the old mapper's behaviour (it returned ``None`` for -such cells). - -**The lookup side is pandas-free (S4 / #89, epic #85).** It is read with pyarrow -and held as numpy arrays plus a sorted key index; attaching metadata to a frame is -a **keyed gather**, not frame algebra, and it never needed a pandas merge. The -input frame is still whatever the caller passes — this class is the *build and -verification* path's object, and its callers hand it DataFrames. What changed is -that the ~880 KiB artifact is no longer materialised as a pandas frame, and the -join no longer depends on the artifact carrying pandas index metadata — which is -what unblocks S5 (#90) making the builder pyarrow-native. - -Produces exactly the 9-column contract enforced at -the delivery's artifact builders and at -views-faoapi ``handlers.py`` (``FAO_PGMDataset._METADATA_COLS``). -""" - -from __future__ import annotations - -import logging -from pathlib import Path -from typing import TYPE_CHECKING - -import numpy as np -import pyarrow as pa - -from views_postprocessing.contract import gaul_lookup - -if TYPE_CHECKING: # pragma: no cover — pandas is in this module's INTERFACE, not its - import pandas as pd # implementation. Callers hand it DataFrames; nothing here - # constructs, reads or joins one. Removing the runtime import is the point of - # S4 (#89): the ~880 KiB lookup is no longer materialised as a pandas frame, and the - # join no longer needs the artifact to carry pandas index metadata. -from views_postprocessing.contract.gaul_schema import METADATA_COLS - -logger = logging.getLogger(__name__) - -# The artifact's identity lives in `gaul_lookup` (#152, C-68) — this alias keeps the -# enricher's own default working without re-deriving the path. -_DEFAULT_LOOKUP = gaul_lookup.LOOKUP_PATH - -#: int64 bounds, named because they are a correctness condition rather than trivia: -#: a value outside them does not raise on cast, it WRAPS, and a wrapped id that is -#: flagged valid is the fabricated value this module forbids. -_INT64_MIN = int(np.iinfo(np.int64).min) -_INT64_MAX = int(np.iinfo(np.int64).max) - -#: Float-comparison bounds. Deliberately NOT the int bounds above: 2**63-1 is odd and -#: unrepresentable in float64, so comparing a float against it rounds up to 2**63 and -#: lets through the very values the bound excludes. 2**63 and -2**63 are both powers -#: of two and exact, so the float check is `>= -2**63` and `< 2**63`. -_INT64_MIN_F = -(2.0**63) -_INT64_MAX_EXCLUSIVE_F = 2.0**63 - -#: The lookup's key column. Named once — the artifact calls it `priogrid_gid`, the -#: wire calls it `priogrid_id` (§5.1), and confusing the two is a silent join failure. -_KEY = "priogrid_gid" - - -class GaulLookupEnricher: - """Merge precomputed GAUL metadata onto an input DataFrame by cell id.""" - - def __init__(self, lookup_path: str | Path | None = None) -> None: - self._lookup_path = Path(lookup_path) if lookup_path else _DEFAULT_LOOKUP - if not self._lookup_path.exists(): - err_msg = ( - f"GAUL lookup table not found at {self._lookup_path}. " - f"Build it with scripts/build_gaul_lookup.py." - ) - logger.error(err_msg) - raise FileNotFoundError(err_msg) - table = gaul_lookup.load(self._lookup_path) - missing = [c for c in (_KEY, *METADATA_COLS) if c not in table.column_names] - if missing: - err_msg = f"Lookup table is missing contract columns: {missing}" - logger.error(err_msg) - raise ValueError(err_msg) - - if table.column(_KEY).null_count: - err_msg = ( - f"GAUL lookup at {self._lookup_path} has null values in {_KEY!r}. A " - "null key cannot identify a cell, and coercing it would make it " - "collide with any unusable id on the query side — the row would then " - "be reported as FOUND and receive another cell's metadata. Rebuild " - "with scripts/build_gaul_lookup.py. (That script does not check the " - "key column either — its null check runs after the key becomes the " - "index, and DataFrame.isna() does not inspect an index. A null key is " - "unreachable there only because the earlier astype('int64') raises. " - "See register C-76.)" - ) - logger.error(err_msg) # ADR-008: logged persistently AND raised - raise ValueError(err_msg) - - if table.num_rows == 0: - err_msg = ( - f"GAUL lookup at {self._lookup_path} is empty. Every cell would gather " - "to null and the delivery would fail downstream complaining about " - "missing metadata rather than about a missing lookup. Rebuild it with " - "scripts/build_gaul_lookup.py." - ) - logger.error(err_msg) # ADR-008: logged persistently AND raised - raise ValueError(err_msg) - - # Sort the whole table once by key, in arrow, so the gather below is a binary - # search per row rather than a scan. `take` reorders every column together — - # doing it column-by-column in python was measurably quadratic and is exactly - # the mistake this comment exists to stop the next person repeating. - keys = table.column(_KEY).to_numpy(zero_copy_only=False).astype(np.int64) - order = np.argsort(keys, kind="stable") - table = table.take(pa.array(order)) - self._keys = keys[order] - # One `to_pylist` per column, not per row. Lists rather than numpy arrays - # because the columns are of mixed kind (float codes, string names) and the - # output is assembled per column anyway. - # - # DTYPE CHANGE, deliberate and measured (S4 / #89). The pandas merge this - # replaced produced `category` name columns, inherited from the artifact's - # dictionary encoding; assigning lists produces `object`. On a 200-row output: - # category 1,795,191 bytes vs object 60,061 — a categorical carries the full - # 64,742-entry dictionary whatever the output size, so this is far lighter for - # the small frames this object actually sees and heavier only past the point - # where the dictionary amortises. The builder's `# names/iso categorical (C-32 - # memory)` note governs the ARTIFACT; it never governed this method's output. - self._values = {col: table.column(col).to_pylist() for col in METADATA_COLS} - self.lookup_version = self._read_version(self._lookup_path) - logger.info( - "Loaded GAUL lookup: %d cells from %s (version=%s)", - len(self._keys), self._lookup_path, self.lookup_version, - ) - - @staticmethod - def _as_cell_ids(gids) -> tuple[np.ndarray, np.ndarray]: - """``(int64 ids, usable mask)`` — DECLARED, never coerced (ADR-003). - - A cell id is an integer. Anything that is not one — a missing value, a - non-integral float, a ``pd.NA`` — is marked **unusable** rather than cast, - and unusable ids gather to null exactly as an unknown gid does. The - downstream gate then sees a hole, which is what it exists for. - - **Why not just cast.** ``np.asarray(gids, dtype=np.int64)`` looks equivalent - and is not, in three ways found in review of #210: - - - a ``NaN`` becomes ``INT64_MIN`` with only a ``RuntimeWarning``. If a lookup - key were ever null it would take the same sentinel, the two would collide, - and the row would be reported FOUND — receiving another cell's metadata. - That is the fabricated value this module's docstring forbids (cf. C-35). - - a non-integral float **truncates silently**: ``54220.000000001`` becomes - ``54220`` and matches a real, *different* cell. Verified against the pandas - merge this replaced: it returns null there, and even warns. A silent wrong - match is strictly worse than the crash it would replace. - - a ``pd.NA`` raises a bare ``TypeError`` from numpy — no log, no - contract-shaped error, unlike every other guard in this file. - """ - arr = np.asarray(gids) - - if arr.dtype.kind in ("i", "u"): - # In range, or it is not the id the caller wrote. uint64 max silently - # wraps to -1 under `astype(np.int64)` and would be flagged VALID. - usable = (arr >= _INT64_MIN) & (arr <= _INT64_MAX) - return np.where(usable, arr, 0).astype(np.int64), usable - - if arr.dtype.kind == "f": - # Finite, integral, AND representable. `1e30` passes the first two and - # then overflows the cast to INT64_MIN — an id nobody asked for, marked - # valid. Same silent-coercion class this method exists to remove. - # STRICTLY below 2**63, not `<= _INT64_MAX`. `_INT64_MAX` is 2**63-1, - # which is odd and NOT representable in float64 — comparing against it - # promotes to float and rounds UP to 2**63, so the bound admitted exactly - # the values it was added to exclude. `float(2**63)` passed every check - # and then wrapped to INT64_MIN, flagged valid. `_INT64_MIN` has no - # equivalent hole: -2**63 is a power of two and exactly representable. - # - # `isfinite` is kept for intent and is SUBSUMED — `inf` fails the upper - # bound, `NaN` fails `arr == rint(arr)` (NaN equals nothing). It is the - # one condition here that survives its own removal, so it is defensive - # rather than load-bearing; the other three each fail the suite. - usable = ( - np.isfinite(arr) - & (arr == np.rint(arr)) - & (arr >= _INT64_MIN_F) - & (arr < _INT64_MAX_EXCLUSIVE_F) - ) - return np.where(usable, arr, 0).astype(np.int64), usable - - # object / pandas-nullable. Accept only values that ALREADY ARE integers. - # - # An earlier draft used `float(value)`, which parses. That accepted the string - # `"54220"` as a cell id — where the pandas merge this replaced raised - # `ValueError: You are trying to merge on object and int64 columns`. A string - # gid column is a declaration error and the old path said so; parsing it is - # inference (ADR-003) and it is the same defect as the drifted float, just - # pointing the other way. `bool` is excluded for the same reason: `True` is - # not cell 1. - ids = np.zeros(len(arr), dtype=np.int64) - usable = np.zeros(len(arr), dtype=bool) - for i, value in enumerate(arr): - # `np.timedelta64` IS an `np.integer` instance — a numpy quirk, and - # `np.datetime64` is not, so the hole was specific to that one type. A - # duration is not a cell id any more than `True` is cell 1. - if isinstance(value, (bool, np.timedelta64)): - continue - if not isinstance(value, (int, np.integer)): - continue - if _INT64_MIN <= int(value) <= _INT64_MAX: - ids[i], usable[i] = int(value), True - return ids, usable - - def _gather(self, gids) -> dict: - """The metadata for each gid, warning about the ones that got none. - - A sorted-key ``searchsorted`` rather than a hash map: the lookup is 64,742 - rows read once per process, and the gather is over the delivery's row count. - Absent gids yield ``None`` in every column — the null the downstream gate - exists to catch, not a sentinel it would pass. An id that is not a usable - cell id is treated the same way: absent, never guessed at. - """ - wanted, usable = self._as_cell_ids(gids) - # `self._keys` is non-empty — __init__ refuses an empty lookup. An earlier draft - # guarded with `(len(self._keys) > 0) & (...)`, which READS as a guard and is - # not one: `&` evaluates both operands, so the index happened regardless and an - # empty lookup raised IndexError from inside the gather rather than ValueError - # from the constructor. Guard where the condition is knowable, not where it bites. - idx = np.clip(np.searchsorted(self._keys, wanted), 0, len(self._keys) - 1) - # `& usable` is NOT belt-and-braces, and no substitute value would make it so. - # Unusable ids substitute to 0, but int64 reserves nothing — the lookup is - # arbitrary data and may legally contain 0, or -1, or INT64_MIN, or whatever - # else one might pick instead. The mask is what carries correctness; the - # substitute is only a placeholder. Pinned by - # `test_an_unusable_id_cannot_match_the_cell_it_was_substituted_with`. - found = (self._keys[idx] == wanted) & usable - out = { - col: [vals[i] if hit else None for i, hit in zip(idx, found)] - for col, vals in self._values.items() - } - - # Warn HERE rather than handing the caller three arrays to reassemble one - # message. `__init__` already co-locates detection and logging at every guard - # (ADR-008); this is the same shape. An earlier draft returned four values, - # three of which existed only to build the string below. - absent = ~found - n_unmapped = int(absent.sum()) - if n_unmapped: - unknown = sorted({int(i) for i, miss, ok in zip(wanted, absent, usable) if miss and ok}) - n_unusable = int((absent & ~usable).sum()) - detail = f"unknown cells {unknown[:20]}" if unknown else "no unknown cells" - if n_unusable: - detail += f"; {n_unusable} row(s) carried no usable cell id" - logger.warning( - "%d/%d rows have no lookup match (will fail validation): %s", - n_unmapped, len(wanted), detail, - ) - return out - - @staticmethod - def _read_version(path: Path) -> str: - """The lookup's build stamp. Delegates to ``gaul_lookup.version`` (#152) — - this was a ``@staticmethod`` that never touched the instance, i.e. a fact - about the artifact, not about the enricher.""" - return gaul_lookup.version(path) - - def enrich_dataframe_with_pg_info( - self, - df: pd.DataFrame, - pg_id_col: str = "priogrid_gid", - time_id_col: str = "month_id", - only_metadata: bool = True, - **ignored_mapper_kwargs, - ) -> pd.DataFrame: - """Return ``df`` with the 9 metadata columns merged in by cell id. - - Signature mirrors the mapper's method so the manager call site changes - minimally. Mapper-only kwargs (``batch_size``, ``use_multiprocessing``, - ``show_progress`` …) are accepted and ignored — a table join needs none - of them — but any unrecognised kwarg is logged at debug so a genuine - caller mistake is not wholly silent. - - Cells absent from the lookup get NaN metadata (fail-loud downstream). - """ - if ignored_mapper_kwargs: - logger.debug( - "GaulLookupEnricher ignoring mapper-only kwargs: %s", - sorted(ignored_mapper_kwargs), - ) - if pg_id_col not in df.columns: - err_msg = f"Column '{pg_id_col}' not found in DataFrame" - logger.error(err_msg) - raise ValueError(err_msg) - - if only_metadata: - keep = [pg_id_col] - if time_id_col in df.columns: - keep.append(time_id_col) - base = df[keep].copy() - else: - base = df.copy() - - gids = base[pg_id_col].to_numpy() - gathered = self._gather(gids) - - merged = base.copy() - for col in METADATA_COLS: - merged[col] = gathered[col] - return merged - - # Convenience alias for new call sites that don't need the legacy name. - enrich = enrich_dataframe_with_pg_info diff --git a/views_postprocessing/contract/frame_extraction.py b/views_postprocessing/contract/frame_extraction.py index ad266fa..2241fbf 100644 --- a/views_postprocessing/contract/frame_extraction.py +++ b/views_postprocessing/contract/frame_extraction.py @@ -1,24 +1,21 @@ """Frame-native representation seam: extract primitives from a views-frames frame. -The **frame counterpart** to ``extraction.py`` (the pandas seam). It returns the *same* -primitives — sets of ints, numpy month arrays — so the representation-free +Returns primitives — sets of ints, numpy month arrays — so the representation-free ``views_postprocessing.delivery`` invariants consume them unchanged. -Per the migration design (epic #85): pandas and views-frames do **not** coexist at runtime, -so these are deliberately **siblings** of the pandas readers in ``extraction.py``, not a -replacement, and there is **no shared ``Extractor`` Protocol** (a polymorphic interface -nobody dispatches on would be speculative — YAGNI/ISP). When the forecast interior moves to a -frame (S3 / #88), the manager calls *these*; the pandas readers stay for the still-pandas -historical path (gated on C-40 / S7). - -Scope: the readers the forecast interior needs — distinct cells and months from the frame's -index. Deliberately **not** here yet (no speculative code): -- the pandas→``(N, S)`` sample-array unpacker — added when rusty_bucket (#143) declares the - layout (the seam will be *told* the layout, never sniff it); -- a frame-native ``unmapped_cell_count`` — geographic metadata lives on the pandas enriched - frame, not the value frame, until the enrichment moves off pandas (S4 / #89); -- a frame-native ``drop_months_above`` — the observed-range clip is on the *historical* - frame, which is gated on the inbound retirement (S7 / #92). +**This module was one of a pair and is now alone.** It was written as the frame-native +sibling of a pandas seam (``extraction.py``), deliberately duplicated rather than +abstracted while both representations were live: pandas and views-frames did not coexist +at runtime, and a shared ``Extractor`` Protocol nobody dispatched on would have been +speculative. That migration is finished — the pandas seam was deleted in #151, and #90 +retired the last module holding a pandas import at all. There is no sibling and no +still-pandas path; what remains is this one seam, and the docstring is corrected rather +than left describing a pair. + +Scope: the readers the forecast interior needs — distinct cells and months from the +frame's index. Deliberately **not** here (no speculative code): a sample-array unpacker, +added when the producer declares its layout — the seam will be *told* the layout, never +sniff it (ADR-003). """ from __future__ import annotations diff --git a/views_postprocessing/contract/gaul_lookup.py b/views_postprocessing/contract/gaul_lookup.py index 23f17bb..7090812 100644 --- a/views_postprocessing/contract/gaul_lookup.py +++ b/views_postprocessing/contract/gaul_lookup.py @@ -7,11 +7,10 @@ times per run** (C-66) — once eagerly into a pandas enricher it never used, then twice more through pyarrow. -The artifact's identity does not belong to the enricher. ``GaulLookupEnricher`` is -one *consumer* of this asset (the pandas merge used by the build/verification path); -the contract delivery is another, and it wants the table and the stamp, not the -merge. Splitting them lets the delivery read the file once, in arrow, with no pandas -anywhere on the path. +The artifact's identity never belonged to that class, which is why splitting them was +the fix: the delivery wants the table and the stamp, not a merge. It reads the file +**once**, in arrow, with no pandas anywhere on the path. The enricher itself was retired +in #90 (register **C-75**) once it was established it had no production caller. Verified by ``tests/test_gaul_lookup_fidelity.py``: the committed artifact matches views-datafactory's authoritative GAUL parquets value-for-value, its key is unique, diff --git a/views_postprocessing/contract/gaul_schema.py b/views_postprocessing/contract/gaul_schema.py index c2b9513..1a49e49 100644 --- a/views_postprocessing/contract/gaul_schema.py +++ b/views_postprocessing/contract/gaul_schema.py @@ -1,6 +1,6 @@ """Single source of truth for the GAUL enrichment schema and PRIO-GRID geometry. -Shared by the production enricher (enrichment.py) and the build/diff tooling +Shared by the delivery's artifact builders and the build/diff tooling (scripts/) so the 9-column contract, the datafactory->contract rename map, and the PRIO-GRID coordinate formula are defined exactly once. diff --git a/views_postprocessing/unfao/managers/README.md b/views_postprocessing/unfao/managers/README.md index 701f8f1..0cc915f 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 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` +- Geography is attached by `contract/historical.py` (the actuals artifact) and `contract/wire/sidecar.py` (the §5 GAUL sidecar), each a keyed gather over the lookup this manager reads once. *(A `GaulLookupEnricher` CIC was linked here until #90; that class had no production caller and was retired — register C-75.)* - ADR-011 (mapper → lookup), ADR-012 (current ontology)