diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 4043806..99292bf 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 | 16 | -| Resolved Concerns | 67 | +| Open Concerns | 15 | +| Resolved Concerns | 68 | --- @@ -440,34 +440,19 @@ Cross-refs: **C-62** (the transitive dependency drag; the other 31 alerts), **C- **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. -**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. +**DECISION 2026-08-04 (#90), which this entry's Owner field required of whoever took it: RETIRE.** -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. +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. -### C-76: `build_gaul_lookup.py` will write an empty lookup without complaint +**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. -| Field | Value | -|-------|-------| -| ID | C-76 | -| Tier | 4 — no silent corruption. A zero-row artifact fails downstream at `historical.build_historical_table`, which raises on cells absent from the lookup. The cost is that it fails **late and confusingly**: the message names missing geography rather than an empty lookup, and the artifact is committed by then. | -| Source | `code-review max` (2026-08-03) — PR #210 second pass, while checking whether the consumer's new guards duplicated a producer guarantee. They do not. | -| Trigger | When `build_gaul_lookup.py` is next run with a new or renamed `--region`, or against a datafactory whose `gaul_admin` parquets have changed shape — check the printed `cells=` count is non-zero before committing the artifact. Nothing else will tell you. | -| Owner | Whoever next runs the builder. It is a two-line guard in a script one person runs by hand, not a scheduling decision. | -| Location | `scripts/build_gaul_lookup.py` — the invariant block at `:246-268` and the write at `:284` | - -The builder's invariant block is thorough about what it checks: index uniqueness (C-59), nulls in the metadata columns, `-1` sentinels in the code columns (C-35). It does not check that any rows survived. A `--region` argument that filters every cell out, or an upstream join that produces nothing, writes a zero-row parquet and prints `cells=0` as though that were a result. - -**Verified 2026-08-03, and the neighbouring worry is NOT real.** The same review asked whether the builder also fails to reject a null key, since `df.isna().sum().sum()` runs *after* `priogrid_gid` becomes the index and `DataFrame.isna()` does not inspect the index. It does not check it — but the null key is unreachable anyway: `df.index.astype("int64")` raises `IntCastingNaNError` two lines earlier. Protection by accident rather than by declaration, which is worth knowing, but not a defect to fix. **Only the empty case is reachable.** - -**Why this was found now.** PR #210 added consumer-side refusals for both an empty lookup and a null key to `GaulLookupEnricher.__init__`, and the review challenged them as duplicating a producer guarantee. Checking established the opposite: for the empty case there is no producer guarantee to duplicate, and for the null key the producer's protection is incidental. The consumer guards stay, and this entry records the producer-side half rather than quietly assuming someone will notice. - -Cross-refs: **C-59** and **C-61** (RESOLVED — the invariant block this sits beside, and the reason it is otherwise thorough), **C-35** (the `-1` defect class it does check for), **C-75** (the consumer whose guards prompted the check), #210. - ---- +**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. --- @@ -679,6 +664,37 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-76: `build_gaul_lookup.py` will write an empty lookup without complaint — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-76 | +| Tier | 4 — no silent corruption. A zero-row artifact fails downstream at `historical.build_historical_table`, which raises on cells absent from the lookup. The cost is that it fails **late and confusingly**: the message names missing geography rather than an empty lookup, and the artifact is committed by then. | +| Source | `code-review max` (2026-08-03) — PR #210 second pass, while checking whether the consumer's new guards duplicated a producer guarantee. They do not. | +| Trigger | When `build_gaul_lookup.py` is next run with a new or renamed `--region`, or against a datafactory whose `gaul_admin` parquets have changed shape — check the printed `cells=` count is non-zero before committing the artifact. Nothing else will tell you. | +| Owner | Whoever next runs the builder. It is a two-line guard in a script one person runs by hand, not a scheduling decision. | +| Location | `scripts/build_gaul_lookup.py` — the invariant block at `:246-268` and the write at `:284` | + +The builder's invariant block is thorough about what it checks: index uniqueness (C-59), nulls in the metadata columns, `-1` sentinels in the code columns (C-35). It does not check that any rows survived. A `--region` argument that filters every cell out, or an upstream join that produces nothing, writes a zero-row parquet and prints `cells=0` as though that were a result. + +**Verified 2026-08-03, and the neighbouring worry is NOT real.** The same review asked whether the builder also fails to reject a null key, since `df.isna().sum().sum()` runs *after* `priogrid_gid` becomes the index and `DataFrame.isna()` does not inspect the index. It does not check it — but the null key is unreachable anyway: `df.index.astype("int64")` raises `IntCastingNaNError` two lines earlier. Protection by accident rather than by declaration, which is worth knowing, but not a defect to fix. **Only the empty case is reachable.** + +**Why this was found now.** PR #210 added consumer-side refusals for both an empty lookup and a null key to `GaulLookupEnricher.__init__`, and the review challenged them as duplicating a producer guarantee. Checking established the opposite: for the empty case there is no producer guarantee to duplicate, and for the null key the producer's protection is incidental. The consumer guards stay, and this entry records the producer-side half rather than quietly assuming someone will notice. + +Cross-refs: **C-59** and **C-61** (RESOLVED — the invariant block this sits beside, and the reason it is otherwise thorough), **C-35** (the `-1` defect class it does check for), **C-75** (the consumer whose guards prompted the check), #210. + +--- + +**RESOLVED 2026-08-04 (#90).** `build()` now refuses a zero-row result: + +> the build produced ZERO cells for region 'land_gaul'. Either the region filtered every cell out, or the join found no overlap between the seven source parquets. An empty lookup is writable and looks like a result; it is not one. + +Pinned by `tests/test_gaul_lookup_fidelity.py::test_builder_refuses_a_build_with_zero_cells`. + +**The guard did not survive its own first test, and that is worth recording.** On an empty table `pa.array([True] * 0)` infers NULL type, so `pc.and_` in the completeness filter raised `ArrowNotImplementedError` *before* the zero-row check could speak — the confusing-late-failure this entry exists to prevent, relocated by one function. The mask is now explicitly `pa.bool_()`. A guard written and not watched fail is decoration (ADR-014 §2); this one was watched, failed for the wrong reason, and was fixed. + +--- + ### C-77: The historical leg names its document from the model path, not from the declared consumer name — and nothing checks the two agree — RESOLVED | Field | Value | diff --git a/scripts/build_gaul_lookup.py b/scripts/build_gaul_lookup.py index 46eb21d..955e89e 100644 --- a/scripts/build_gaul_lookup.py +++ b/scripts/build_gaul_lookup.py @@ -28,8 +28,9 @@ import os from pathlib import Path -import pandas as pd +import numpy as np import pyarrow as pa +import pyarrow.compute as pc import pyarrow.parquet as pq from views_postprocessing.contract.gaul_schema import ( @@ -76,21 +77,21 @@ def _resolve_datafactory() -> Path: return sibling -def _load_source(datafactory: Path) -> pd.DataFrame: - """Join the 7 GAUL parquets on gid into one wide frame (source names).""" +def _load_source(datafactory: Path) -> pa.Table: + """Join the 7 GAUL parquets on gid into one wide table (source names). + + Arrow throughout since #90 — the sources are parquet, the output is parquet, and + pandas was only ever the thing in the middle. The join is an inner join on ``gid``: + a cell missing from any one source has no complete row to contribute, and the + completeness filter in ``build`` would drop it anyway. + """ gaul_dir = datafactory / "data" / "raw" / "gaul_admin" - frames = {} + table = None for src_col in SOURCE_RENAME: - t = pq.read_table(gaul_dir / f"{src_col}.parquet") - s = pd.Series( - t.column("value").to_pylist(), - index=t.column("gid").to_pylist(), - name=src_col, - ) - frames[src_col] = s - df = pd.DataFrame(frames) - df.index.name = "gid" - return df + one = pq.read_table(gaul_dir / f"{src_col}.parquet").select(["gid", "value"]) + one = one.rename_columns(["gid", src_col]) + table = one if table is None else table.join(one, keys="gid", join_type="inner") + return table def _region_gids(datafactory: Path, region: str) -> set[int] | None: @@ -197,46 +198,82 @@ def _provenance(datafactory: Path, *, datasets: tuple[str, ...]) -> dict: return out -def build(datafactory: Path, region: str, out: Path) -> pd.DataFrame: +def build(datafactory: Path, region: str, out: Path) -> pa.Table: + """Build the lookup and write it. Arrow end to end (#90). + + **What must not change, and is asserted by the fidelity suite:** the nine declared + columns in ``METADATA_COLS`` order with ``priogrid_gid`` last, the four name columns + dictionary-encoded, codes ``int64``, coordinates ``float64``, rows sorted by cell id, + and the declared metadata keys. The delivered artifact is read as arrow by its only + consumer (``contract.gaul_lookup.load``), so those are the observable contract. + + **One deliberate difference from the pandas build:** the written file no longer + carries a ``pandas`` schema-metadata blob. It described an index that arrow does not + have, nothing in this repository reads the lookup with pandas, and reproducing it + would have meant keeping knowledge of pandas' metadata format in the one script this + change exists to remove pandas from. + """ src = _load_source(datafactory) # Optionally restrict to a region's cell set. region_gids = _region_gids(datafactory, region) if region_gids is not None: - src = src.loc[src.index.intersection(sorted(region_gids))] - - # Rename to the contract names. - df = src.rename(columns=SOURCE_RENAME) - - # Keep only fully-complete cells. Incomplete cells must NOT enter the - # lookup: an unknown/incomplete gid then merges to null downstream and the - # manager's _validate() gate crashes (fail-loud) instead of shipping a hole - # or a -1 sentinel. Never carry -1 / "" as a value. - complete = pd.Series(True, index=df.index) - for c in CODE_COLS: - complete &= df[c].notna() & (df[c] != -1) - for c in NAME_COLS: - complete &= df[c].notna() & (df[c].astype(str).str.len() > 0) - dropped = int((~complete).sum()) - df = df[complete].copy() - - # Coordinates from the gid (no geometry needed). - gids = df.index.to_numpy() - df["pg_xcoord"] = [xcoord(int(g)) for g in gids] - df["pg_ycoord"] = [ycoord(int(g)) for g in gids] + wanted = pa.array(sorted(region_gids), type=src.column("gid").type) + src = src.filter(pc.is_in(src.column("gid"), value_set=wanted)) + + # Rename to the contract names, keeping `gid` alongside. + table = src.rename_columns( + ["gid"] + [SOURCE_RENAME[c] for c in src.column_names if c != "gid"] + ) if src.column_names[0] == "gid" else src.rename_columns( + [SOURCE_RENAME.get(c, c) for c in src.column_names] + ) - # dtypes: codes numeric, coords float64, names/iso categorical (C-32 memory). + # Keep only fully-complete cells. Incomplete cells must NOT enter the lookup: an + # unknown/incomplete gid then gathers to null downstream and the delivery's null + # gate fails loud instead of shipping a hole or a -1 sentinel. Never carry -1 / "". + # Typed explicitly: on an empty table `pa.array([True] * 0)` infers NULL type, and + # `pc.and_` then raises ArrowNotImplementedError before the zero-row guard below can + # say anything useful. Found by the guard's own mutation test. + complete = pa.array([True] * table.num_rows, type=pa.bool_()) for c in CODE_COLS: - df[c] = df[c].astype("int64") - for c in COORD_COLS: - df[c] = df[c].astype("float64") + col = table.column(c) + complete = pc.and_(complete, pc.and_(pc.is_valid(col), pc.not_equal(col, -1))) for c in NAME_COLS: - df[c] = df[c].astype("category") + col = table.column(c).cast(pa.string()) + complete = pc.and_( + complete, + pc.and_(pc.is_valid(col), pc.greater(pc.utf8_length(col), 0)), + ) + dropped = int(pc.sum(pc.invert(complete)).as_py() or 0) + table = table.filter(complete) + + # Coordinates from the gid (no geometry needed). Vectorised over the id array + # rather than a Python loop per row — the formula is the declared one either way. + gids = table.column("gid").to_numpy(zero_copy_only=False).astype(np.int64) + table = table.append_column( + "pg_xcoord", pa.array([xcoord(int(g)) for g in gids], type=pa.float64()) + ).append_column( + "pg_ycoord", pa.array([ycoord(int(g)) for g in gids], type=pa.float64()) + ) - df = df[METADATA_COLS] - df.index = df.index.astype("int64") - df.index.name = "priogrid_gid" - df = df.sort_index() + # dtypes: codes int64, coords float64, names dictionary-encoded. The dictionary + # encoding is the artifact's shipped type — it was `category` under pandas and the + # committed file carries dictionary. + cast = {c: pa.int64() for c in CODE_COLS} + cast.update({c: pa.float64() for c in COORD_COLS}) + columns, names = [], [] + for c in METADATA_COLS: + col = table.column(c) + if c in NAME_COLS: + col = pc.dictionary_encode(col.cast(pa.string())) + else: + col = col.cast(cast[c]) + columns.append(col) + names.append(c) + columns.append(table.column("gid").cast(pa.int64())) + names.append("priogrid_gid") + table = pa.Table.from_arrays(columns, names=names) + table = table.sort_by([("priogrid_gid", "ascending")]) # Hard invariants — the lookup must be clean by construction. # @@ -245,14 +282,20 @@ def build(datafactory: Path, region: str, out: Path) -> pd.DataFrame: # identical on disk (register C-61). The -1 check in particular has no downstream # backstop — -1 is non-null, so every gate in the delivery chain would pass it # straight through to FAO, which is exactly the resolved C-35 defect recurring. - if not df.index.is_unique: - dupes = df.index[df.index.duplicated()].unique().tolist() + key = table.column("priogrid_gid") + if len(pc.unique(key)) != table.num_rows: + counts = key.value_counts() + dupes = [ + counts.field("values")[i].as_py() + for i in range(len(counts)) + if counts.field("counts")[i].as_py() > 1 + ] raise LookupBuildError( - f"{len(dupes)} duplicate gid(s) in the lookup index: {dupes[:10]}. " - "A duplicated key multiplies rows through the enricher's left-merge with " - "every value non-null, so no downstream gate can see it (C-59)." + f"{len(dupes)} duplicate gid(s) in the lookup key: {dupes[:10]}. " + "A duplicated key multiplies rows through a keyed gather with every value " + "non-null, so no downstream gate can see it (C-59)." ) - n_null = int(df.isna().sum().sum()) + n_null = sum(table.column(c).null_count for c in table.column_names) if n_null: raise LookupBuildError( f"lookup contains {n_null} null value(s); only fully-complete cells may " @@ -260,20 +303,31 @@ def build(datafactory: Path, region: str, out: Path) -> pd.DataFrame: "downstream." ) for c in CODE_COLS: - n_sentinel = int((df[c] == -1).sum()) + n_sentinel = int(pc.sum(pc.equal(table.column(c), -1)).as_py() or 0) if n_sentinel: raise LookupBuildError( f"{c} contains {n_sentinel} -1 sentinel(s). -1 is non-null, so it would " "reach FAO through every gate as a country/admin code (cf. C-35)." ) + # Register C-76: a build that filtered every cell out is not a result. Downstream + # this fails late and confusingly — `build_historical_table` raises about missing + # geography rather than about an empty lookup, and by then the artifact is + # committed. Two lines here, at the moment a human is present. + if table.num_rows == 0: + raise LookupBuildError( + f"the build produced ZERO cells for region {region!r}. Either the region " + "filtered every cell out, or the join found no overlap between the seven " + "source parquets. An empty lookup is writable and looks like a result; it " + "is not one." + ) out.parent.mkdir(parents=True, exist_ok=True) - table = pa.Table.from_pandas(df, preserve_index=True) - meta = dict(table.schema.metadata or {}) - meta[b"adr"] = b"ADR-011" - meta[b"region"] = region.encode() - meta[b"n_cells"] = str(len(df)).encode() - meta[b"n_dropped_incomplete"] = str(dropped).encode() + meta = { + b"adr": b"ADR-011", + b"region": region.encode(), + b"n_cells": str(table.num_rows).encode(), + b"n_dropped_incomplete": str(dropped).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. @@ -282,9 +336,9 @@ def build(datafactory: Path, region: str, out: Path) -> pd.DataFrame: table = table.replace_schema_metadata(meta) pq.write_table(table, out) - print(f"region={region} cells={len(df):,} dropped_incomplete={dropped:,}") + print(f"region={region} cells={table.num_rows:,} dropped_incomplete={dropped:,}") print(f"wrote {out} ({out.stat().st_size/1e6:.2f} MB)") - return df + return table def main() -> None: diff --git a/tests/test_gaul_lookup_fidelity.py b/tests/test_gaul_lookup_fidelity.py index 870672d..821ddfb 100644 --- a/tests/test_gaul_lookup_fidelity.py +++ b/tests/test_gaul_lookup_fidelity.py @@ -37,6 +37,7 @@ from pathlib import Path import numpy as np +import pyarrow as pa import pyarrow.parquet as pq import pytest @@ -302,22 +303,24 @@ def test_coordinate_formula_matches_every_priogrid_cell(): # ── the builder's own invariants must not be vacuous ──────────────────────── -def _synthetic_source(n: int = 6): - """A minimal well-formed source frame in the producer's column vocabulary.""" - import pandas as pd +def _synthetic_source(n: int = 6) -> pa.Table: + """A minimal well-formed source table in the producer's column vocabulary. - gids = list(range(1, n + 1)) - return pd.DataFrame( + Arrow since #90 — ``_load_source`` returns a table with a ``gid`` COLUMN rather than + a frame with a ``gid`` index, because arrow has no index. The builder's contract with + this seam is the column vocabulary, which is unchanged. + """ + return pa.table( { - "gaul0_code": [10] * n, - "gaul0_name": ["Country"] * n, - "gaul1_code": [20] * n, - "gaul1_name": ["Admin1"] * n, - "gaul2_code": [30] * n, - "gaul2_name": ["Admin2"] * n, - "iso3_code": ["ABC"] * n, - }, - index=pd.Index(gids, name="gid"), + "gid": pa.array(range(1, n + 1), type=pa.int64()), + "gaul0_code": pa.array([10] * n, type=pa.int64()), + "gaul0_name": pa.array(["Country"] * n), + "gaul1_code": pa.array([20] * n, type=pa.int64()), + "gaul1_name": pa.array(["Admin1"] * n), + "gaul2_code": pa.array([30] * n, type=pa.int64()), + "gaul2_name": pa.array(["Admin2"] * n), + "iso3_code": pa.array(["ABC"] * n), + } ) @@ -349,16 +352,16 @@ def _build_with(monkeypatch, source, tmp_path, provenance=None): def test_builder_rejects_a_duplicate_gid(monkeypatch, tmp_path): """C-59: prove the uniqueness guard is not vacuous. - Note the guard is only *reachable* with ``region="all"``. Under a region filter - pandas' ``Index.intersection`` silently de-duplicates first — an accidental + Note the guard is only *reachable* with ``region="all"``. Under a region filter the + old pandas build de-duplicated silently via ``Index.intersection`` — an accidental protection, not a declared one, which is why the explicit raise still earns its - place. See the C-59 narrative for the corrected exposure analysis. + place. Since #90 the filter is ``pc.is_in``, which does **not** de-duplicate, so the + raise is the only thing standing there. See the C-59 narrative. """ - import pandas as pd import scripts.build_gaul_lookup as builder source = _synthetic_source() - duplicated = pd.concat([source, source.iloc[[2]]]) + duplicated = pa.concat_tables([source, source.slice(2, 1)]) with pytest.raises(builder.LookupBuildError, match="duplicate gid"): _build_with(monkeypatch, duplicated, tmp_path) @@ -379,17 +382,48 @@ def test_a_sentinel_code_is_dropped_rather_than_shipped(monkeypatch, tmp_path): pandas internals, and a test that fragile is worse than the invariant it guards. """ source = _synthetic_source() - source.loc[3, "gaul1_code"] = -1 + codes = source.column("gaul1_code").to_pylist() + codes[2] = -1 + source = source.set_column( + source.column_names.index("gaul1_code"), "gaul1_code", + pa.array(codes, type=pa.int64()), + ) result = _build_with(monkeypatch, source, tmp_path) - assert 3 not in result.index, "the -1 cell must be dropped from the lookup" - assert len(result) == 5 + assert 3 not in result.column("priogrid_gid").to_pylist(), ( + "the -1 cell must be dropped from the lookup" + ) + assert result.num_rows == 5 + + +def test_builder_refuses_a_build_with_zero_cells(monkeypatch, tmp_path): + """Register C-76 — an empty lookup is writable and looks like a result. + + A ``--region`` that filters every cell out, or a join with no overlap across the + seven sources, produced a zero-row parquet and printed ``cells=0`` as though that + were an outcome. It failed later and confusingly: ``build_historical_table`` raises + about *missing geography*, not about an empty lookup, and by then the artifact is + committed. + + The first draft of the guard did not survive its own test. On an empty table + ``pa.array([True] * 0)`` infers NULL type, so ``pc.and_`` raised + ``ArrowNotImplementedError`` in the completeness filter — before the zero-row check + could say anything. The mask is now typed, and this asserts the refusal is the one a + human can act on. + """ + import scripts.build_gaul_lookup as builder + + empty = _synthetic_source(0) + with pytest.raises(builder.LookupBuildError, match="ZERO cells"): + _build_with(monkeypatch, empty, tmp_path) def test_builder_accepts_a_clean_source(monkeypatch, tmp_path): """The guards must not reject well-formed input.""" result = _build_with(monkeypatch, _synthetic_source(), tmp_path) - assert len(result) == 6 - assert list(result.columns) == METADATA_COLS + assert result.num_rows == 6 + # The nine declared columns in normative order, with the key last — the artifact's + # shipped shape, which #90 had to preserve exactly while changing what builds it. + assert result.column_names == METADATA_COLS + ["priogrid_gid"] @_needs_datafactory