diff --git a/docs/ADRs/011_replace_runtime_mapper_with_precomputed_lookup.md b/docs/ADRs/011_replace_runtime_mapper_with_precomputed_lookup.md index a19dc01..e2cc9a7 100644 --- a/docs/ADRs/011_replace_runtime_mapper_with_precomputed_lookup.md +++ b/docs/ADRs/011_replace_runtime_mapper_with_precomputed_lookup.md @@ -1,5 +1,7 @@ # ADR-011: Replace Runtime Spatial Mapper with Precomputed Lookup Table + + **Status:** Accepted **Date:** 2026-06-02 **Deciders:** Project maintainers (PRIO MD&D Team) diff --git a/docs/ADRs/012_revised_ontology.md b/docs/ADRs/012_revised_ontology.md index 9bc11d4..9b05a14 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**. + 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). @@ -58,9 +58,9 @@ described the cut-a-repo case; it now says which is which. | Category | Purpose | Authority | Stability | |----------|---------|-----------|-----------| | **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 | +| **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 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 | +| **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 | @@ -70,7 +70,7 @@ described the cut-a-repo case; it now says which is which. **Two claims this ADR made until 2026-08-01, both now corrected rather than quietly dropped** (register C-67). It called the manager *"the **thin** `UNFAOPostProcessorManager`"* when it was -636 lines holding two of everything, and it called `unfao/extraction.py` *"the **single** +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 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 @@ -129,7 +129,7 @@ Seam" wording will need a light touch (the seam stays; its internals change). ## Amendment 2026-07-31 — forecast identity re-homed to the wire layer (#150, epic #148) -**`views_postprocessing/delivery/identity.py` is retired.** The "Delivery Invariants" row +**`views_postprocessing/delivery/identity.py` is retired.** The "Delivery Invariants" row above listed *forecast identity* among the authoritative rules; that module no longer exists, and this amendment records where the rule went so a reader of the row is not looking for deleted code. diff --git a/docs/ADRs/013_sampled_forecast_wire_contract.md b/docs/ADRs/013_sampled_forecast_wire_contract.md index 3f8e57c..769007e 100644 --- a/docs/ADRs/013_sampled_forecast_wire_contract.md +++ b/docs/ADRs/013_sampled_forecast_wire_contract.md @@ -1120,7 +1120,7 @@ record execution progress against it. **4.73 GB peak, 108 shards, 5 min, zero store calls** (interlock held). **(B) the historical path is pandas-free** (#126): actuals fetched as a `views_frames.FeatureFrame` (the frame path's FIRST production consumer — - C-40's gate lifted), artifact built by `unfao/historical.py` via pyarrow — + C-40's gate lifted), artifact built by `contract/historical.py` via pyarrow — reader-level parity with a legacy characterization golden proven through faoapi's own reader semantics. Two ghosts found in the legacy artifact and deliberately exorcised (faoapi reader verified safe on both): junk `row`/`col` @@ -1142,8 +1142,8 @@ record execution progress against it. passed at 128 draws, sidecar + manifest staged, zero store calls. - **2026-07-20 — HOP-B SINK LEG SHIPPED (epic #105 complete; upload-disabled).** The contract's missing middle exists in fixture-proven code: - `unfao/wire/` (naming, header, shard, sidecar, run_manifest, source_selection, - sink) + `unfao/product.py` + `delivery/parity.py`, wired into the manager + `contract/wire/` (naming, header, shard, sidecar, run_manifest, source_selection, + sink) + each partner's `product.py` + `delivery/parity.py`, wired into the manager behind an explicit declared `wire_contract` launch key. Settled by shipping: **§4.2a's configuration home is `unfao/product.py`**; **§5.2's parity invariant is `delivery/parity.py`**; the §11.4 upload interlock is live in @@ -1177,7 +1177,7 @@ record execution progress against it. merged 2026-07-15 and run-0 uploaded 2026-07-27, so the sequencing constraint was satisfied. The legacy reader it protected has now been **deleted**: this repo reads forecasts only through the contract path (manifest selection, - `unfao/wire/source_selection.py`) and historical actuals only as a + `contract/wire/source_selection.py`) and historical actuals only as a `views_frames.FeatureFrame` (#126). **The retired guard, recorded here because the code that carried it is gone.** @@ -1204,7 +1204,7 @@ record execution progress against it. this ADR should know the surrounding code changed shape and the wire did not. **What changed.** The pandas delivery this contract replaced was retired (#149) along - with the config fork that silently selected it; `delivery/identity.py` was retired + with the config fork that silently selected it; `delivery/identity.py` was retired because the contract path enforces declared identity **per shard header** instead (#150, §4.2a); the duplicate representation seam collapsed (#151); the GAUL lookup became one artifact read once (#152); and the partner-neutral machinery — the whole diff --git a/docs/CICs/UNFAOPostProcessorManager.md b/docs/CICs/UNFAOPostProcessorManager.md index 0178739..af7b513 100644 --- a/docs/CICs/UNFAOPostProcessorManager.md +++ b/docs/CICs/UNFAOPostProcessorManager.md @@ -9,8 +9,8 @@ --- > **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 +> 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. diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 7a0818c..1a725a4 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 | 12 | -| Resolved Concerns | 71 | +| Open Concerns | 11 | +| Resolved Concerns | 72 | --- @@ -421,31 +421,6 @@ Cross-refs: **C-62** (the transitive dependency drag; the other 31 alerts), **C- --- -### 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 | @@ -577,6 +552,45 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-80: The doc-accuracy scan exempts ADRs and CICs — the two artifact classes that define the contracts — RESOLVED + +| 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. + +**RESOLVED 2026-08-05 (B5).** ADRs and CICs are now scanned, and the design was chosen by measurement rather than by argument. + +**What the measurement said.** A path-resolution check over ADRs would have fired **29** times, and inspecting them showed most were correct history, other repositories' files, or paths inside URLs — the cry-wolf outcome this entry predicted, confirmed before building it. The curated deleted-symbol list was the narrower instrument: **22** hits over ADRs, **0** over CICs. Fourteen of the 22 were in ADR-011 alone. + +**Three of those hits were real.** ADR-013 still cited `unfao/historical.py`, `unfao/wire/` and `unfao/wire/source_selection.py` — all moved to `contract/` by #153, all fixed here. Three genuine defects hiding among five markable ones is a workable ratio, and it is the ratio that justified turning the scan on. + +**Two escapes, both declared rather than inferred.** Line-scoped `legacy-ok` for an isolated historical mention; a new file-level `` for a document whose *subject* is a retirement. ADR-011 is the case that earned it — it **is** the decision to remove the runtime mapper, so its subject appears fourteen times, correctly. A second guard pins the set of file-level exemptions to that one document, so adding another shows up in a diff. + +**And a check nothing else could have made.** `test_a_cic_does_not_name_a_collaborator_its_class_never_calls` asserts that a class a CIC names is actually referenced by the class it documents — the `GaulLookupEnricher` failure, which no path check and no symbol list would have caught at the time, because the class existed and the paths resolved. Exception types are excluded: a *collaborator* is something the class reaches for, an *exception* something that passes through, and the first draft flagged three exceptions the manager legitimately propagates. + +**The mutation campaign found a miss in the previous change.** Reintroducing the exact `GaulLookupEnricher` sentence did **not** fail — the collaborator check only sees classes that still exist, and #90/C-75 had deleted that one. The real gap was that the deletion never extended the deleted-symbol list, which that list's own comment demands in as many words: *"A deletion PR that does not extend this regex has not finished."* Extended here; the reintroduction now fails. Four further historical mentions written yesterday were flagged by the extension and marked. + +Cross-refs: **C-75** (the retirement whose CIC error motivated this, and whose PR the mutation test caught short), **C-74** and **C-78** (guards narrower than their declared surface), **C-82** (the register's own prose, still unscanned), ADR-014 §2 and §3. + +--- + ### C-83: A queryset that fails to import is reported as a queryset that declares the wrong format — RESOLVED | Field | Value | diff --git a/tests/test_doc_accuracy.py b/tests/test_doc_accuracy.py index 1669d72..ace3fbc 100644 --- a/tests/test_doc_accuracy.py +++ b/tests/test_doc_accuracy.py @@ -54,6 +54,13 @@ # #150 / #151 / #153 — modules retired or moved out of `unfao/` by epic #148 r"delivery/identity", r"unfao/extraction", r"unfao/frames", r"unfao/historical", r"unfao/gaul_schema", r"unfao/wire", + # #90 / C-75 — the enricher retired once it was shown to have no production + # caller. Added 2026-08-05, and late: the deletion PR did not extend this list, + # which is exactly what the paragraph above says a deletion PR has not finished + # doing. Found by the mutation test for the new CIC-collaborator guard, which + # could NOT catch a reintroduced mention — that guard only sees classes that + # still exist, and this one no longer does. + r"GaulLookupEnricher", r"enrich_dataframe_with_pg_info", r"contract/enrichment", # #149 — pipeline-core's pandas container, no longer referenced by this repo. # Negative lookbehind: `FAO_PGMDataset` is views-faoapi's class and is live. # It inherits IGNORECASE, so `fao_PGMDataset` is spared too — no such spelling @@ -93,6 +100,158 @@ def _deleted_symbol_offenders(docs: list[Path]) -> list[str]: return offenders +#: A document may declare, once at the top, that its SUBJECT is a retirement — after +#: which the deleted-symbol scan skips it wholesale. +#: +#: ADR-011 is the case this exists for: it is the decision to *replace* the runtime +#: mapper, so `PriogridCountryMapper`, `geopandas` and `mapping.py` appear fourteen +#: times in it, every one correctly. Marking fourteen lines would be noise pretending +#: to be rigour; marking the document once is the same claim, made where a reader sees +#: it. Per-line ``legacy-ok`` remains the tool for an isolated mention in a document +#: that is otherwise about the present. +_RETIREMENT_DOC_MARKER = "` if its subject IS the retirement. " + "If it is not history, it is a stale path and the document is wrong." + ) + + +def test_the_retirement_marker_is_not_used_to_silence_a_live_document(): + """An escape hatch nobody bounds is just a disabled check. + + A whole-document exemption is the strongest thing available here, so the set of + documents holding one is pinned. Adding another is a deliberate act that shows up in + a diff, rather than a quiet way to stop a guard complaining. + """ + marked = sorted( + d.name for d in _governance_docs() if _RETIREMENT_DOC_MARKER in d.read_text() + ) + assert marked == ["011_replace_runtime_mapper_with_precomputed_lookup.md"], ( + f"documents exempted from the deleted-symbol scan: {marked}. Only a document " + "whose SUBJECT is a retirement earns this. If a present-tense document needs " + "it, the document is describing code that no longer exists." + ) + + def test_living_docs_have_no_deleted_symbol_references(): offenders = _deleted_symbol_offenders(_living_docs()) assert not offenders, "deleted-symbol references in living docs:\n" + "\n".join(offenders) diff --git a/views_postprocessing/unfao/managers/README.md b/views_postprocessing/unfao/managers/README.md index 0cc915f..ac7d4b6 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 -- 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.)* +- 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)