diff --git a/docs/CICs/GaulLookupEnricher.md b/docs/CICs/GaulLookupEnricher.md index a9d7ef0..4d0938e 100644 --- a/docs/CICs/GaulLookupEnricher.md +++ b/docs/CICs/GaulLookupEnricher.md @@ -62,9 +62,16 @@ views-datafactory area-majority join), so this class does only a table join. - Output: the input frame (or, with `only_metadata=True`, just `pg_id_col` + `time_id_col`) left-merged with the 9 metadata columns. - Public attribute: `lookup_version` — a short, stampable id read from the - lookup's embedded provenance at construction (`@`, or - `"unknown"` if the lookup carries none). The manager stamps it on each - delivery so a delivery is traceable to the exact lookup build. + 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. diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index b0c1c7c..457a3cc 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-02 | | Total Concerns | 74 | -| Open Concerns | 21 | -| Resolved Concerns | 53 | +| Open Concerns | 20 | +| Resolved Concerns | 54 | --- @@ -71,7 +71,7 @@ covered a single open entry (see Historical clusters below). **The lesson, and it generalises past this cluster.** C-43's residual said the forward-check *"was a one-off session result, not a standing guarantee"* — and the fix was to attach it to something the interpreter runs. The entries then reproduced the identical error one level up: they stated their closing conditions in prose and attached them to nothing. S2 therefore added a closing-condition check to `test_register_integrity.py`. **A guarantee needs a check, and that applies to the register's own guarantees too.** -**Remaining in this cluster:** **C-60** (the flat declared `lookup_version` key — S5 / #186) and **C-46** (the hardcoded datafactory path — S7 / #188). Both unblocked, both in epic #181. +**C-60 closed 2026-08-02 (S5 / #186)** — the flat declared key landed and the committed artifact was rebuilt, metadata-only. **Remaining in this cluster: C-46 alone** (the hardcoded datafactory path — S7 / #188), which is the last thing keeping the cluster open. ### Cluster L: The won migration was never cleaned up **Root cause:** the frame-native contract path replaced the pandas path and **won** — run-0 delivered global-land on 2026-07-27 and FAO has been served from it since. The replaced path was deliberately kept behind a config fork "until run 0 proves the contract path live" (C-40) and was then never removed. Everything below is residue of that one omission, not independent defects. @@ -454,26 +454,6 @@ Cross-refs: C-57 (registry drift — the most likely way a coordinate goes wrong --- -### C-60: The lookup provenance stamp reaches into the producer's ledger schema and degrades to `"unknown"` on a bare except - -| Field | Value | -|-------|-------| -| ID | C-60 | -| Tier | 3 | -| Source | `expert-code-review` (2026-07-31) — Ousterhout lens (information leakage / silent degradation) | -| Trigger | When views-datafactory renames or restructures its ingestion-ledger entries (`dataset` key, `content_digest` field, or the `land_gaul_region` entry name) — verify `lookup_version` still resolves to a real value rather than the string `"unknown"`; nothing fails if it does not | -| Location | `views_postprocessing/unfao/enrichment.py:61-79` (`_read_version`); written at `scripts/build_gaul_lookup.py:89-107` (`_provenance`) and `:163-164`; consumed as the C-15 provenance field via the manager's delivery description | - -`_read_version` reconstructs the stamp by traversing three levels of the producer's schema — parquet metadata → `source_provenance` JSON → `land_gaul_region` → `content_digest` — and wraps the traversal in `except (ValueError, AttributeError): pass`, returning `"unknown"` when anything along the path is absent. This is a **declare-don't-infer violation at the consumer**: the value that ties a delivered artifact to the exact lookup build (C-15's traceability provenance) can silently become a placeholder, and no gate notices. - -No wrong data results — this is a traceability failure, not a correctness one → **Tier 3**. But it defeats the specific question C-15 exists to answer *after* a suspect delivery ("which lookup produced this?"), and C-22 has no correction procedure that could compensate. - -**Mitigation:** have `build_gaul_lookup.py` write a **flat, declared `lookup_version` key** into the parquet metadata, and have `_read_version` read that one key and **raise** if absent. The consumer stops knowing the producer's nested ledger schema, and the stamp stops being able to vanish quietly. Separately worth stamping `lookup_version` into the sidecar's own parquet metadata so a delivered artifact is self-describing without the store document. - -Cross-refs: C-15 (the provenance this field serves), C-22 (the recall process that would need it), C-57 (the same class — a cross-repo fact this repo reads without a way to detect drift), **Cluster K**. - ---- - ### C-62: The pinned pipeline-core release still installs geopandas and torch into a repo that architecturally excised them | Field | Value | @@ -694,6 +674,38 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-60: The lookup provenance stamp reaches into the producer's ledger schema and degrades to `"unknown"` on a bare except — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-60 | +| Resolved | 2026-08-02 | +| Resolution | **Closed by S5 (#186).** The builder composes the stamp and writes one flat `lookup_version` key; `contract/gaul_lookup.version` reads that key and **raises** `LookupVersionError` (logged first, ADR-008) when it is absent. The three-level traversal of views-datafactory's ledger shape, the bare `except … pass` and the `"unknown"` branch are all gone — pinned by `tests/test_gaul_lookup_access.py`, which asserts by **AST** rather than by word-matching that the module imports no `json` and holds zero exception handlers. An untraceable *build* now fails at build time, where a human is present, instead of surfacing as a placeholder in production. + +**The committed artifact was rebuilt, metadata-only.** 64,742 rows, column order and dtypes unchanged, a content hash over every column's values identical, and the stamp resolves to `land_gaul@f74d3b2b` — the same string the old traversal produced, so no delivered provenance changes meaning. ADR-013 untouched: `contract_version` 1.5, `tests/fixtures/` byte-identical (`build_sidecar` constructs a fresh table, so lookup metadata cannot reach the wire — verified empirically, sidecar schema metadata is `[]`). + +**⚠ The fix reintroduced this entry's own defect class, caught in review before merge.** The first draft read `provenance.get("land_gaul_region") or provenance.get("gaul_admin_area_majority")`. Since `_provenance` takes no region, `land_gaul_region` is present for *every* build — so `--region all` would have been stamped `all@f74d3b2b`, the land_gaul region definition's digest, on a global artifact: authoritative-looking, wrong, silent. Replaced by a declared `stamp_dataset(region)` with no fallback; a region whose ledger entry is absent is **refused**, not substituted. Verified: `all@272cdb01`, `land_gaul@f74d3b2b`, `africa_me_legacy` → raises. Worth keeping visible — the reflex that produced C-60 reappeared while fixing C-60. + +**Residual, deliberately deferred:** stamping `lookup_version` into the **sidecar's own** parquet metadata, so a delivered artifact is self-describing without the store document. Not done here because it changes wire bytes and therefore `contract_version` — an ADR-013 amendment with three repos to notify. Named trigger: **the next ADR-013 version bump**, whatever prompts it. + +**Also recorded from review:** the ingestion ledger is append-only and the **last entry per dataset wins** (14 `gaul_admin_area_majority` entries, 2 `land_gaul_region`). Harmless while provenance was decorative; load-bearing now that the stamp raises, so it is stated in `_provenance` rather than left implicit — the same shape **C-73** was opened for on the forecast path, intended here and now said. `docs/CICs/GaulLookupEnricher.md` still documented the `"unknown"` degrade and was corrected. | +| Tier | 3 | +| Source | `expert-code-review` (2026-07-31) — Ousterhout lens (information leakage / silent degradation) | +| Trigger | When views-datafactory renames or restructures its ingestion-ledger entries (`dataset` key, `content_digest` field, or the `land_gaul_region` entry name) — verify `lookup_version` still resolves to a real value rather than the string `"unknown"`; nothing fails if it does not | +| Location | `views_postprocessing/unfao/enrichment.py:61-79` (`_read_version`); written at `scripts/build_gaul_lookup.py:89-107` (`_provenance`) and `:163-164`; consumed as the C-15 provenance field via the manager's delivery description | + +`_read_version` reconstructs the stamp by traversing three levels of the producer's schema — parquet metadata → `source_provenance` JSON → `land_gaul_region` → `content_digest` — and wraps the traversal in `except (ValueError, AttributeError): pass`, returning `"unknown"` when anything along the path is absent. This is a **declare-don't-infer violation at the consumer**: the value that ties a delivered artifact to the exact lookup build (C-15's traceability provenance) can silently become a placeholder, and no gate notices. + +No wrong data results — this is a traceability failure, not a correctness one → **Tier 3**. But it defeats the specific question C-15 exists to answer *after* a suspect delivery ("which lookup produced this?"), and C-22 has no correction procedure that could compensate. + +**Mitigation:** have `build_gaul_lookup.py` write a **flat, declared `lookup_version` key** into the parquet metadata, and have `_read_version` read that one key and **raise** if absent. The consumer stops knowing the producer's nested ledger schema, and the stamp stops being able to vanish quietly. Separately worth stamping `lookup_version` into the sidecar's own parquet metadata so a delivered artifact is self-describing without the store document. + +Cross-refs: C-15 (the provenance this field serves), C-22 (the recall process that would need it), C-57 (the same class — a cross-repo fact this repo reads without a way to detect drift), **Cluster K**. + +--- + +--- + ### C-03: Test coverage gaps in manager validation and the enrich→validate path — RESOLVED | Field | Value | diff --git a/scripts/build_gaul_lookup.py b/scripts/build_gaul_lookup.py index 7b60b44..b488cdb 100644 --- a/scripts/build_gaul_lookup.py +++ b/scripts/build_gaul_lookup.py @@ -90,8 +90,79 @@ def _region_gids(datafactory: Path, region: str) -> set[int] | None: return set(json.loads(path.read_text())) -def _provenance(datafactory: Path) -> dict: - """Pull the land_gaul ledger entry for traceability (best-effort).""" +#: The ledger dataset naming the whole GAUL area-majority join, used to stamp an +#: unregionalised (``--region all``) build. +AREA_MAJORITY_DATASET = "gaul_admin_area_majority" + +#: Length of the digest carried in a stamp. Declared so producer and consumer agree. +DIGEST_CHARS = 8 + + +def stamp_dataset(region: str) -> str: + """DECLARED: which ledger dataset's digest stamps this region's build. + + One rule, no fallback. ``--region all`` is stamped by the area-majority join that + produced every value; a regional build is stamped by that region's own definition + entry, because the region determines which rows exist and a change to it changes + the artifact. + + **Why not "whichever entry happens to be present" (register C-60, review of #186).** + The first draft read ``land_gaul_region or gaul_admin_area_majority``. Since + ``_provenance`` does not take a region, ``land_gaul_region`` is present for *every* + build — so ``--region all`` would have been stamped ``all@f74d3b2b``, the land_gaul + region definition's digest, on a global artifact. Authoritative-looking and wrong, + silently: the exact defect class C-60 exists to close, reintroduced one level up. + A region with no ledger entry is now refused, not substituted. + """ + return AREA_MAJORITY_DATASET if region == "all" else f"{region}_region" + + +def _lookup_version(region: str, provenance: dict) -> str: + """Compose the DECLARED build stamp: ``@``. + + This is the value written as a flat ``lookup_version`` key and read back verbatim + by ``contract.gaul_lookup.version`` — register **C-60**. Composing it here is the + point: the builder is the only thing holding both the region and the producer's + ledger, so the consumer stops needing to know views-datafactory's schema in order + to answer "which lookup produced this delivery?". + + **Refuses an untraceable build.** ``_provenance`` is best-effort by design — a + missing or reshaped ledger yields ``{}``. Previously that produced the string + ``"unknown"`` at *delivery* time, silently, in the one field C-15 exists to answer + after a suspect delivery. Failing here moves the error to the moment a human is + present to fix it: the build. + """ + dataset = stamp_dataset(region) + digest = str((provenance.get(dataset) or {}).get("content_digest", "")) + if len(digest) < DIGEST_CHARS: + raise LookupBuildError( + f"cannot compose a lookup_version for region {region!r}: the datafactory " + f"ingestion ledger has no usable content_digest for {dataset!r} " + f"(provenance/gaul_admin/ingestion_ledger.jsonl; need at least " + f"{DIGEST_CHARS} characters, got {len(digest)}). An artifact without a " + "declared version cannot be traced back to the build that made it, which " + "is what register C-15 needs after a suspect delivery. Rebuild against a " + "datafactory checkout whose ledger declares that dataset — this build is " + "NOT stamped from some other region's entry." + ) + return f"{region}@{digest[:DIGEST_CHARS]}" + + +def _provenance(datafactory: Path, *, datasets: tuple[str, ...]) -> dict: + """Pull the named ledger entries for traceability (best-effort). + + Kept richer than the stamp — it carries timestamps and the upstream GAUL digest, + which are worth having. What changed in C-60 is that the STAMP is no longer + derived from it by the consumer; see ``_lookup_version``. + + **The ledger is append-only and the LAST entry per dataset wins.** The real file + carries 14 ``gaul_admin_area_majority`` entries and 2 ``land_gaul_region`` ones; + this loop overwrites, so the most recent ingestion is what stamps the build. That + was harmless while provenance was decorative. Since C-60 the stamp *raises* on its + absence, so the selection rule is load-bearing and is stated here rather than left + to be inferred from the loop. (Recency-over-a-broad-match is the shape register + **C-73** was opened for on the forecast path; here it is intended, and now said.) + """ ledger = (datafactory / "provenance" / "gaul_admin" / "ingestion_ledger.jsonl") out = {} @@ -101,8 +172,7 @@ def _provenance(datafactory: Path) -> dict: e = json.loads(line) except json.JSONDecodeError: continue - if e.get("dataset") in ("land_gaul_region", - "gaul_admin_area_majority"): + if e.get("dataset") in datasets: out[e["dataset"]] = { k: e[k] for k in ("content_digest", "source_gaul_digest", "timestamp") @@ -188,8 +258,11 @@ def build(datafactory: Path, region: str, out: Path) -> pd.DataFrame: meta[b"region"] = region.encode() meta[b"n_cells"] = str(len(df)).encode() meta[b"n_dropped_incomplete"] = str(dropped).encode() - meta[b"source_provenance"] = json.dumps( - _provenance(datafactory)).encode() + prov = _provenance(datafactory, datasets=(AREA_MAJORITY_DATASET, stamp_dataset(region))) + # The DECLARED stamp: one flat key, composed here, read verbatim by the consumer + # (C-60). Key order in parquet metadata carries no meaning; this is a dict. + meta[b"lookup_version"] = _lookup_version(region, prov).encode() + meta[b"source_provenance"] = json.dumps(prov).encode() table = table.replace_schema_metadata(meta) pq.write_table(table, out) diff --git a/tests/test_gaul_lookup_access.py b/tests/test_gaul_lookup_access.py index e6be468..d0de6c6 100644 --- a/tests/test_gaul_lookup_access.py +++ b/tests/test_gaul_lookup_access.py @@ -11,9 +11,15 @@ read per run, and the read *injected* into both consumers rather than fetched by each. """ +import ast +import importlib +import inspect +import logging from pathlib import Path import pyarrow as pa +import pyarrow.parquet as pq +import pytest from views_postprocessing.contract import gaul_lookup @@ -34,13 +40,80 @@ def test_load_returns_the_arrow_table(): assert "priogrid_gid" in table.column_names -def test_version_reports_the_build_stamp(): +def test_version_reports_the_declared_build_stamp(): + """C-60: the stamp is read from one declared key, not reconstructed. + + This used to assert ``stamp != "unknown"`` — a real check while ``version()`` + could *return* that placeholder. It no longer can: the branch is gone and the + function raises instead. What is worth pinning now is the shape a delivery + traces by. + """ stamp = gaul_lookup.version() - assert stamp != "unknown", ( - "the lookup's provenance stamp did not resolve — a delivery would ship " - "untraceable provenance (register C-60)" + assert "@" in stamp, f"expected `@`, got {stamp!r}" + region, digest = stamp.split("@", 1) + assert region and len(digest) == 8, stamp + + +def test_version_raises_rather_than_degrading_when_the_key_is_absent(tmp_path): + """The load-bearing half of C-60's fix, without which it is unproven. + + An artifact built before the declared key existed — or by a future builder that + forgets it — must stop the delivery, not hand it a placeholder that reads like a + version. ``"unknown"`` in a provenance record is worse than a crash: it is + written, stored and shipped, and nothing downstream can tell it from a real stamp. + """ + unstamped = tmp_path / "no_version.parquet" + pq.write_table(pa.table({"priogrid_gid": pa.array([1, 2, 3])}), unstamped) + with pytest.raises(gaul_lookup.LookupVersionError, match="lookup_version"): + gaul_lookup.version(unstamped) + + +def test_version_logs_before_it_raises(tmp_path, caplog): + """ADR-008:48/51, matching the shape S1 (#182) gave the entry validators.""" + unstamped = tmp_path / "no_version.parquet" + pq.write_table(pa.table({"priogrid_gid": pa.array([1])}), unstamped) + with caplog.at_level(logging.ERROR): + with pytest.raises(gaul_lookup.LookupVersionError): + gaul_lookup.version(unstamped) + errors = [r for r in caplog.records if r.levelno >= logging.ERROR] + assert len(errors) == 1 and "lookup_version" in errors[0].getMessage() + + +def test_version_no_longer_knows_the_producers_ledger_schema(): + """The declare-don't-infer half: the consumer stopped traversing upstream shape. + + ``version()`` reached three levels into views-datafactory's ingestion ledger — + ``source_provenance`` → ``land_gaul_region`` → ``content_digest`` — inside a bare + ``except … pass``. A rename upstream made every delivery untraceable with no + signal. The builder composes the stamp now; this reads one key. + + **Tested by mechanism, not by word.** An earlier draft scanned the file for those + key names and failed on the docstring above, which names them in order to explain + the fix. Prose that records why a thing changed is not the thing. So this asserts + the two capabilities the traversal required and the degrade depended on: JSON + decoding of the producer's blob, and an exception handler that swallows. Neither + can be present without the defect being reachable, and neither is triggered by a + sentence. + """ + module = importlib.import_module("views_postprocessing.contract.gaul_lookup") + tree = ast.parse(inspect.getsource(module)) + + imports = { + alias.name + for node in ast.walk(tree) + if isinstance(node, ast.Import) + for alias in node.names + } + assert "json" not in imports, ( + "gaul_lookup imports json again — decoding the producer's provenance blob is " + "how the consumer came to know views-datafactory's schema (register C-60)." + ) + + handlers = [n for n in ast.walk(tree) if isinstance(n, ast.ExceptHandler)] + assert not handlers, ( + f"gaul_lookup has {len(handlers)} exception handler(s). The stamp degraded to " + '"unknown" through a bare `except … pass`; this module now declares or raises.' ) - assert "@" in stamp # `@` def test_the_delivery_reads_the_lookup_exactly_once(monkeypatch): diff --git a/tests/test_gaul_lookup_fidelity.py b/tests/test_gaul_lookup_fidelity.py index 78fe669..a451188 100644 --- a/tests/test_gaul_lookup_fidelity.py +++ b/tests/test_gaul_lookup_fidelity.py @@ -315,15 +315,28 @@ def _synthetic_source(n: int = 6): ) -def _build_with(monkeypatch, source, tmp_path): +#: A resolvable ledger entry, so a synthetic build can reach the invariant block. +#: Keyed on the area-majority dataset because ``_build_with`` builds ``region="all"``, +#: whose DECLARED stamp source is that entry — see ``builder.stamp_dataset``. +_SYNTHETIC_PROVENANCE = {"gaul_admin_area_majority": {"content_digest": "0123456789abcdef"}} + + +def _build_with(monkeypatch, source, tmp_path, provenance=None): """Run the builder against ``source``, bypassing the datafactory entirely. - ``region="all"`` short-circuits ``_region_gids`` (returns None), and - ``_provenance`` is best-effort, so no checkout is required. + ``region="all"`` short-circuits ``_region_gids`` (returns None). Provenance is + stubbed for the same reason the source is: since C-60 the builder **refuses** a + build whose ledger yields no digest, so a synthetic run needs a synthetic ledger + to reach the invariants these tests are about. That refusal is not bypassed — + it is proven directly by ``test_builder_refuses_a_build_it_cannot_stamp``. """ import scripts.build_gaul_lookup as builder monkeypatch.setattr(builder, "_load_source", lambda _df: source) + monkeypatch.setattr( + builder, "_provenance", + lambda _df, **_kw: _SYNTHETIC_PROVENANCE if provenance is None else provenance, + ) return builder.build(Path("/nonexistent"), "all", tmp_path / "lookup.parquet") @@ -384,3 +397,77 @@ def test_coord_dtypes_are_wire_stable(lookup): ) for col in COORD_COLS: assert lookup.schema.field(col).type == "double", f"{col} must be float64" + + +# ── the declared build stamp (S5 / #186, register C-60) ───────────────────── + +def test_builder_refuses_a_build_it_cannot_stamp(monkeypatch, tmp_path): + """C-60: an untraceable artifact must fail at BUILD, not degrade at delivery. + + Before this, an unresolvable ledger produced the string ``"unknown"`` in the + delivery's provenance — silently, in the one field C-15 exists to answer after a + suspect delivery. The record was still written; it just stopped meaning anything. + Moving the failure to the build puts it where a human is present to fix it. + """ + import scripts.build_gaul_lookup as builder + + with pytest.raises(builder.LookupBuildError, match="lookup_version"): + _build_with(monkeypatch, _synthetic_source(), tmp_path, provenance={}) + + +def test_the_builder_writes_the_declared_version_key(monkeypatch, tmp_path): + """The producer composes the stamp; nothing downstream reconstructs it.""" + import pyarrow.parquet as pq + + out = tmp_path / "lookup.parquet" + _build_with(monkeypatch, _synthetic_source(), tmp_path) + meta = {k.decode(): v.decode() for k, v in (pq.read_metadata(out).metadata or {}).items()} + assert meta["lookup_version"] == "all@01234567", ( + "the builder must write a flat, declared lookup_version — @" + ) + + +def test_the_committed_artifact_declares_its_version(): + """The artifact this repo actually ships, not a synthetic one.""" + from views_postprocessing.contract import gaul_lookup + + stamp = gaul_lookup.version() + assert stamp.startswith("land_gaul@"), stamp + assert "@" in stamp and len(stamp.split("@")[1]) == 8, ( + f"expected @<8-char digest>, got {stamp!r}" + ) + + +def test_the_stamp_source_is_declared_per_region_never_borrowed(): + """C-60, review of #186: the first draft borrowed whatever entry was present. + + ``_provenance`` does not take a region, so ``land_gaul_region`` is in the blob for + *every* build. A ``land_gaul_region or gaul_admin_area_majority`` fallback would + therefore have stamped ``--region all`` as ``all@`` — an + authoritative-looking claim about a global artifact, sourced from one region's + definition. That is the defect class C-60 exists to close, one level up. + """ + import scripts.build_gaul_lookup as builder + + assert builder.stamp_dataset("all") == builder.AREA_MAJORITY_DATASET + assert builder.stamp_dataset("land_gaul") == "land_gaul_region" + assert builder.stamp_dataset("africa_me_legacy") == "africa_me_legacy_region" + + # A region whose declared entry is absent is REFUSED, not silently substituted — + # even though another dataset's digest is sitting right there in the blob. + blob = {"land_gaul_region": {"content_digest": "f74d3b2bdeadbeef"}} + assert builder._lookup_version("land_gaul", blob) == "land_gaul@f74d3b2b" + with pytest.raises(builder.LookupBuildError, match="africa_me_legacy_region"): + builder._lookup_version("africa_me_legacy", blob) + + +def test_a_short_digest_is_refused_rather_than_truncated_silently(): + """Producer and consumer must agree on what a valid stamp is. + + ``[:8]`` alone would emit ``land_gaul@abcd`` from a 4-char digest — accepted by + the builder, rejected by ``test_the_committed_artifact_declares_its_version``. + """ + import scripts.build_gaul_lookup as builder + + with pytest.raises(builder.LookupBuildError, match="at least 8 characters"): + builder._lookup_version("land_gaul", {"land_gaul_region": {"content_digest": "abcd"}}) diff --git a/views_postprocessing/contract/gaul_lookup.py b/views_postprocessing/contract/gaul_lookup.py index 098d21f..6907f31 100644 --- a/views_postprocessing/contract/gaul_lookup.py +++ b/views_postprocessing/contract/gaul_lookup.py @@ -20,41 +20,63 @@ from __future__ import annotations -import json +import logging from pathlib import Path import pyarrow as pa import pyarrow.parquet as pq +logger = logging.getLogger(__name__) + +#: The single declared metadata key carrying the artifact's build stamp. +#: Written by ``scripts/build_gaul_lookup.py``; read verbatim here (C-60). +VERSION_KEY = "lookup_version" + + +class LookupVersionError(RuntimeError): + """The lookup artifact does not declare which build produced it.""" + #: The committed ADR-011 lookup. Declared public — three modules depend on it, and a #: leading underscore said the opposite (C-68). LOOKUP_PATH: Path = Path(__file__).resolve().parent.parent / "data" / "gaul_lookup.parquet" def version(path: Path | None = None) -> str: - """A short, stampable build id for the lookup, for delivery provenance (C-15). - - Format: ``@`` (e.g. ``land_gaul@f74d3b2b``) so a - delivery can be traced to the exact lookup build. - - **Known limitation, tracked as register C-60:** this reaches three levels into - views-datafactory's ingestion-ledger shape (``source_provenance`` → - ``land_gaul_region`` → ``content_digest``) and returns ``"unknown"`` if any level - is absent or reshaped — so the traceability stamp can silently degrade. Moved - verbatim from ``GaulLookupEnricher._read_version`` in #152 **without behaviour - change**; fixing it means having the builder write a flat declared key and - raising here when it is missing, which is C-60's own scope. + """The lookup's DECLARED build stamp, for delivery provenance (C-15). + + Reads one flat key — ``lookup_version`` — written by the builder, which is the + only thing holding both the region and the producer's ledger. Format: + ``@`` (e.g. ``land_gaul@f74d3b2b``), so a delivery + can be traced to the exact lookup build that made it. + + Raises: + LookupVersionError: if the artifact declares no version. It does not + degrade to a placeholder — see below. + + **Why it raises (register C-60).** This function used to reconstruct the stamp + by traversing three levels of views-datafactory's ingestion-ledger shape + (``source_provenance`` → ``land_gaul_region`` → ``content_digest``) inside a + bare ``except … pass``, returning the string ``"unknown"`` when any level was + absent or reshaped. A rename upstream would therefore have made every delivery + untraceable, silently, in the one field C-15 exists to answer *after* a suspect + delivery — and no gate would have noticed, because a provenance record was + still written; it just stopped meaning anything. + + The consumer no longer knows the producer's schema. The builder declares; this + reads. ADR-003. """ - meta = pq.read_metadata(path or LOOKUP_PATH).metadata or {} - meta = {k.decode(): v.decode() for k, v in meta.items()} - region = meta.get("region", "?") - digest = "?" - try: - prov = json.loads(meta.get("source_provenance", "{}")) - digest = (prov.get("land_gaul_region", {}).get("content_digest", "?"))[:8] - except (ValueError, AttributeError): - pass - return "unknown" if region == "?" and digest == "?" else f"{region}@{digest}" + raw = pq.read_metadata(path or LOOKUP_PATH).metadata or {} + declared = raw.get(VERSION_KEY.encode()) + if not declared: + err_msg = ( + f"the GAUL lookup at {path or LOOKUP_PATH} declares no {VERSION_KEY!r} — " + "a delivery built from it could not be traced back to the build that " + "produced it (register C-15). Rebuild the artifact with " + "scripts/build_gaul_lookup.py, which writes the key (C-60)." + ) + logger.error(err_msg) # ADR-008: logged persistently AND raised + raise LookupVersionError(err_msg) + return declared.decode() def load(path: Path | None = None) -> pa.Table: diff --git a/views_postprocessing/data/gaul_lookup.parquet b/views_postprocessing/data/gaul_lookup.parquet index 9c28ccb..307ac49 100644 Binary files a/views_postprocessing/data/gaul_lookup.parquet and b/views_postprocessing/data/gaul_lookup.parquet differ