Repository navigation
S5 — The lookup declares its own version instead of inferring it (C-60) #186
Description
Part of epic #181. Closes register C-60 (Tier 3).
Background
gaul_lookup.version() produces the stamp that ties a delivered artifact to the exact lookup build that made it — C-15's traceability provenance, the field you would need after a suspect delivery to answer "which lookup produced this?".
It gets that stamp by reaching three levels into views-datafactory's ingestion-ledger shape:
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}"Today it returns land_gaul@f74d3b2b. If views-datafactory renames the land_gaul_region ledger entry, moves content_digest, or restructures source_provenance, this returns the string "unknown" — silently, through a bare except … pass, and nothing in the delivery notices. The provenance record still gets written; it just stops meaning anything.
That is a declare-don't-infer violation at the consumer: the reader knows the producer's private schema, and degradation is indistinguishable from success. gaul_lookup.version's own docstring already flags it as a known limitation and names this issue's scope as the fix.
De-risking already done — read this before scoping the change
The obvious fear is that changing the lookup artifact changes the wire. It does not, and this was verified empirically rather than assumed:
>>> sc = build_sidecar(gaul_lookup.load(), gids)
>>> sorted(k.decode() for k in (sc.schema.metadata or {}))
[]sidecar.build_sidecar constructs a fresh pa.table(columns) from extracted arrays (wire/sidecar.py:58-73). It does not replace_schema_metadata, and pa.table() starts with none — so the lookup's parquet key-value metadata cannot reach the sidecar. lookup_version flows to delivery/provenance.py:46 → the store document's delivery description, not to any wire artifact.
Consequences for this story:
- ADR-013
contract_versiondoes not move. Thetests/fixtures/wire_contract/bytes are untouched. - The change is safe to make in one PR.
- Verify this claim again in the PR rather than trusting this paragraph —
git diff --stat origin/main -- tests/fixtures/must be empty andtests/test_wire_fixture.pymust be green.
Work
Producer — scripts/build_gaul_lookup.py:
Write a flat, declared lookup_version key into the parquet metadata alongside the existing adr / region / n_cells keys, holding the composed value (<region>@<short digest>). Keep source_provenance — it is richer than the stamp and is worth carrying — but stop making the stamp derivable only from it.
The builder is the right place to compose it: it already has region and _provenance(datafactory) in hand, and it is the only thing that knows both.
Decide and record: what should the builder do when _provenance finds no ledger entry? _provenance is documented "best-effort" and returns {} if the ledger file is absent. A build with no resolvable digest produces an artifact that cannot be traced. Per this repo's fail-loud idiom, that should be a LookupBuildError at build time, not an "unknown" at delivery time — the build is the moment a human is present to fix it. State the choice in the docstring.
Consumer — views_postprocessing/contract/gaul_lookup.py:
version() reads the single lookup_version key and raises if it is absent or empty. Delete the three-level traversal, the bare except, and the "unknown" branch. Log before raising (ADR-008) — the same shape S1 gives appwrite_env. Update the docstring: the C-60 "known limitation" paragraph is replaced by a statement of the declared key.
The committed artifact — views_postprocessing/data/gaul_lookup.parquet:
Must be rebuilt, because the existing one has no lookup_version key and the new reader raises without it. views-datafactory is available locally (../views-datafactory, 7 GAUL parquets present):
python scripts/build_gaul_lookup.py --datafactory ../views-datafactory --region land_gaul
The rebuild must be metadata-only in effect: same 64,742 rows, same values, same column order. Prove it, do not assume it — tests/test_gaul_lookup_fidelity.py (18 tests) is exactly the instrument, and it must stay green against the rebuilt artifact.
Consider also stamping lookup_version into the sidecar's own parquet metadata so a delivered artifact is self-describing. Not in this story — it would change wire bytes and therefore contract_version, and that is an ADR-013 amendment with three repos to notify. Note it in the register as C-60's deliberate residual if it still seems worth doing.
Acceptance criteria
gaul_lookup.version()has no"unknown"branch and no bareexcept.- It raises, with a log record, when the declared key is absent.
- The committed artifact carries a flat
lookup_versionkey. - The rebuilt artifact is value-identical to the old one: 64,742 rows, 9 columns, same order, same values.
tests/test_gaul_lookup_fidelity.py— 18 passed, unchanged.git diff --stat origin/main -- tests/fixtures/is empty;contract_versionis still"1.5".- The builder's behaviour on an unresolvable ledger is decided, implemented, and documented.
Testing
New tests (home: tests/test_gaul_lookup_access.py, which already owns version()):
test_version_reads_the_declared_key— round-trip a tiny synthetic parquet with the key set.test_version_raises_when_the_declared_key_is_absent— write one without it, assert the raise. This is the test that makes the whole story load-bearing; without it the fix is unproven.test_version_logs_before_raising— ADR-008, matching S1's shape.- Builder-side: assert the written artifact carries the key, and (if you chose fail-loud) that an unresolvable ledger raises.
Existing test to watch: tests/test_gaul_lookup_access.py:37 already asserts version() != "unknown" — it should now be impossible rather than merely true. Re-point or keep as a belt-and-braces check, your call, but say which in the PR.
Validation:
ruff check .
pytest -q tests/test_gaul_lookup_fidelity.py tests/test_gaul_lookup_access.py tests/test_wire_sidecar.py tests/test_wire_fixture.py
pytest -q
git diff --stat origin/main -- tests/fixtures/ # must be empty
Dependencies
None. Blocks S7 and S8.
Files
scripts/build_gaul_lookup.py:93-108(_provenance),:184-196(metadata write)views_postprocessing/contract/gaul_lookup.py:36-62(version)views_postprocessing/data/gaul_lookup.parquet— rebuiltviews_postprocessing/delivery/provenance.py:20,46— the consumer of the stampviews_postprocessing/unfao/managers/unfao.py:387— where it enters a deliveryviews_postprocessing/contract/wire/sidecar.py:58-73— why the wire is unaffected
Activity
chore(register): S5 #186 — resolve C-60, and record that the fix repr…
S5 (#186) — the lookup declares its own version instead of the consum…
Polichinel commentedon Aug 2, 2026
Landed in #201. C-60 closed.
The builder composes the stamp and writes one flat
lookup_versionkey;version()reads it and raises when absent, logging first. The three-level ledger traversal, the bareexcept … passand the"unknown"branch are gone — pinned by AST, not word-matching, so a docstring explaining the fix cannot satisfy the guard.The rebuilt artifact is metadata-only, proven: 64,742 rows, column order and dtypes unchanged, a content hash over every column's values identical, and the stamp still resolves to
land_gaul@f74d3b2b— the same string the old traversal produced, so no delivered provenance changes meaning.contract_versionstays 1.5 and the fixtures are byte-identical.Review caught the fix reintroducing C-60's own defect. The first draft read
land_gaul_region or gaul_admin_area_majority— and since_provenancetakes no region,--region allwould have been stampedall@f74d3b2b, the land_gaul region definition's digest, on a global artifact. Replaced by a declaredstamp_dataset(region)with no fallback; an undeclared region is refused, not substituted.Residual deferred behind a named trigger rather than a vague later: stamping
lookup_versioninto the sidecar's own metadata would change wire bytes and thereforecontract_version, so it waits for the next ADR-013 bump.Register: 74 concerns, 20 open. Cluster K is down to C-46 alone.