Skip to content

S5 — The lookup declares its own version instead of inferring it (C-60) #186

Description

@Polichinel

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_version does not move. The tests/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 and tests/test_wire_fixture.py must be green.

Work

Producerscripts/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.

Consumerviews_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 artifactviews_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 bare except.
  • It raises, with a log record, when the declared key is absent.
  • The committed artifact carries a flat lookup_version key.
  • 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_version is 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 — rebuilt
  • views_postprocessing/delivery/provenance.py:20,46 — the consumer of the stamp
  • views_postprocessing/unfao/managers/unfao.py:387 — where it enters a delivery
  • views_postprocessing/contract/wire/sidecar.py:58-73 — why the wire is unaffected

Metadata

Metadata

Assignees

No one assigned

    Labels

    implementationCode implementation workstoryA single reviewable unit of an epictestingTest/parity/validation work

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions