From b10de6f17c7fca02abbda515ef67c0e2e9e7640c Mon Sep 17 00:00:00 2001 From: Polichinl Date: Sun, 2 Aug 2026 03:44:03 +0200 Subject: [PATCH] =?UTF-8?q?test(validation):=20S4=20#185=20=E2=80=94=20ret?= =?UTF-8?q?ire=20the=20=5Fvalidate=20replica;=20test=20the=20gate=20where?= =?UTF-8?q?=20it=20fires?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/test_validation.py defined its own validate_dataframe() and ran 43 parametrised cases against it. Forty-three tests, zero production code — a closed loop that could not have failed if the real gate were deleted outright. Its header claimed "the logic tested matches unfao.py:_validate() exactly". False since #149, which stopped _validate null-gating entirely; its docstring now reads "Neither payload is null-gated here." It also carried REQUIRED_METADATA_COLS as a nine-element literal — the hand copy of the contract that C-70 was resolved to make single-source. Replaced by 14 tests of contract/historical.assert_metadata_complete, the code that actually gates a delivery, parametrised over the IMPORTED METADATA_COLS so a column added to gaul_schema.COLUMNS is gated automatically. Plus source-scan pins that the gate has not drifted back into _validate and that the manager still calls it. Mutation-tested: narrowing the gate to a single column fails 9 of the 14. The old suite passed that same mutation untouched. The behaviour C-01 fixed is preserved in full; it is simply asserted against the function that performs it. C-03 closes. Residual 1 (the replica) is gone. Residual 2 (the enrich->validate end-to-end path) is RELOCATED to #18 by number, per the convention that a relocation is not complete until the destination exists and is cited. Every leg is covered — enrichment, artifact build, reader parity, invariants on primitives, the wire e2e, and now the null-gate. What remains is the manager ORCHESTRATING them, which needs pipeline-core and a production-like Appwrite environment that þing-02 D2 forbids. That gap already had two trackers; a third here was noise. Three `assert False` campaign probes retired because the findings they assert are discharged: 3.2's "validation uses replica not real code", 4.3's "filter_cols change not detected", and 4.3's "enrichment test checks only 5 of 9" — the last verified before removal, since test_enrichment.py asserts list(lookup.columns) == METADATA_COLS, exact equality on all nine, then iterates the full set twice more. C-36's own resolution predicted this: "the pure-assert False probes don't test the live condition, so a fixed finding won't auto-flip." A discharged probe must be retired by hand or it permanently asserts a defect that no longer exists. Each removal leaves a dated note; the modules stay, because the campaign record is the point. Review caught the story closing C-03 while the CIC still cited it as open, next to a now-false "replica tests" — the exact drift class, in the contract document. Corrected to name #18 and þing-02 D2. Co-Authored-By: Claude Opus 5 (1M context) --- docs/CICs/UNFAOPostProcessorManager.md | 2 +- reports/technical_risk_register.md | 55 +++-- tests/test_falsification_campaign_3_2.py | 23 +- tests/test_falsification_campaign_4_3.py | 42 ++-- tests/test_validation.py | 268 ++++++++++++----------- 5 files changed, 198 insertions(+), 192 deletions(-) diff --git a/docs/CICs/UNFAOPostProcessorManager.md b/docs/CICs/UNFAOPostProcessorManager.md index cf281c9..2629c93 100644 --- a/docs/CICs/UNFAOPostProcessorManager.md +++ b/docs/CICs/UNFAOPostProcessorManager.md @@ -145,7 +145,7 @@ manager._save() - **Beige tests:** Missing ensemble name in config; None environment variables; empty forecast bucket; DataFrames with unexpected index structure - **Red tests:** Corrupted parquet downloads; network timeouts during upload; DataFrames where all cells map to None (all-ocean input) -Currently: the manager cannot be instantiated without `views-pipeline-core`, so its stage logic is covered by **source-scan and replica tests** — `tests/test_validation.py`, `tests/test_launch_config.py` (the refusals), `tests/test_gaul_lookup_access.py` (one lookup read, threaded). `tests/test_append_metadata.py` was deleted in #149 with the method it mirrored. A full end-to-end test against the live manager (C-03) still requires a production-like environment. +Currently: the manager cannot be instantiated without `views-pipeline-core`, so its stage logic is covered by **source-scan and seam tests** — `tests/test_validation.py` (the metadata null-gate, tested against `contract/historical.assert_metadata_complete` where it fires, plus pins that it has not drifted back into `_validate`), `tests/test_launch_config.py` (the refusals), `tests/test_gaul_lookup_access.py` (one lookup read, threaded). There is no longer a replica of any manager method: `tests/test_validation.py` held one until S4 (#185) and it had diverged from `_validate` since #149, while `tests/test_append_metadata.py` was deleted in #149 with the method it mirrored. A full end-to-end test against the **live manager** still requires a production-like environment and is tracked as **#18**; þing-02 **D2** forbids integration tests against the production Appwrite project, no non-production one existing. The input-integrity guards (S0–S6, epic #51) are representation-free invariants in `views_postprocessing/delivery/` that the manager **calls** (never inherits). Each has primitives unit tests — `tests/test_delivery_coverage.py` (S1/S4), `tests/test_delivery_observed_range.py` (S2), `tests/test_provenance.py` (S5), `tests/test_frame_extraction.py` (the seam), `tests/test_store_metadata.py` (store identity) — and `tests/test_input_integrity_e2e.py` drives the invariants on **primitives**, which is how the manager calls them. S3's forecast-identity rule moved to the wire layer (#150) and is covered by `tests/test_wire_source_selection.py`. The design contract (representation-free, called-not-inherited) is pinned by `tests/test_input_integrity_design_contract.py`. diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 8e5a23e..b0c1c7c 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 | 22 | -| Resolved Concerns | 52 | +| Open Concerns | 21 | +| Resolved Concerns | 53 | --- @@ -113,24 +113,6 @@ that indexes only deleted code is noise. ## Open Concerns -### C-03: Test coverage gaps in manager validation and the enrich→validate path - -| Field | Value | -|-------|-------| -| ID | C-03 | -| Tier | 3 | -| Source | `repo-assimilation` (2026-06-02), `test-review` (2026-06-02) | -| Trigger | When modifying the manager's `_validate()` or the enricher, verify that the test suite covers the changed behavior — end-to-end coverage across the enrich→validate path is still absent | -| Location | `tests/test_validation.py`, `views_postprocessing/unfao/managers/unfao.py` | - -Initial state was zero test coverage. A 73-test suite was written (2026-06-02) covering the (now-deleted) mapper's core guarantees and the validation logic (missing columns, null rejection, error messages). Remaining gaps after the mapper removal: (1) the validation tests replicate `_validate()` logic in a standalone function because `views-pipeline-core` is unavailable in test environments — if the real `_validate()` diverges, tests pass while production fails; (2) no end-to-end test enriches through `GaulLookupEnricher` then validates through the manager. - -Tier recalibrated from 2 to 3 during review-rr (2026-06-02): the gap is maintainability (test-code divergence, missing integration path), not structural fragility. - -**Update 2026-06-24:** narrowed with the mapper deletion (C-39, PR #42). The mapper-coverage dimension is gone with the mapper (`tests/test_mapping.py` deleted; the determinism/cache/shapefile/`ThreadPoolExecutor` gaps no longer exist). Two manager-side gaps remain: the standalone `_validate()` replica and the missing enrich→validate end-to-end test. - ---- - ### C-07: Undeclared direct runtime dependencies in pyproject.toml | Field | Value | @@ -711,6 +693,37 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), --- ## Resolved Concerns + +### C-03: Test coverage gaps in manager validation and the enrich→validate path — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-03 | +| Resolved | 2026-08-02 | +| Resolution | **Closed by S4 (#185): one of its two residuals is gone, the other is relocated to a tracker that already exists.** + +**Residual 1 — the `_validate()` replica — REMOVED.** `tests/test_validation.py` defined its own `validate_dataframe` and then tested *that*: forty-odd parametrised cases exercising a function declared twelve lines above them and **no production code at all**. A closed loop that could not fail while the delivery broke. Its header claimed *"the logic tested matches unfao.py:_validate() exactly"* — false since **#149**, which stopped `_validate` null-gating entirely (its docstring now says *"Neither payload is null-gated here"*). It also carried `REQUIRED_METADATA_COLS` as a nine-element literal, a hand copy of the contract **C-70** was resolved to make single-source. + +Replaced by tests of `contract/historical.assert_metadata_complete` — the code that actually gates a delivery — parametrised over the **imported** `METADATA_COLS`, plus source-scan pins that the gate stays at build time and is still invoked. Verified 2026-08-02: `pytest -q tests/test_validation.py` → **14 passed**. Mutation-tested: narrowing the gate to a single column fails **9 of 14**; the old suite passed that mutation untouched, because it was not testing the gate. + +**Residual 2 — the enrich→validate end-to-end test — RELOCATED to #18**, per the Register Conventions' relocation rule (a relocation is not complete until the destination exists and is cited by number). Every *leg* is now covered — enrichment (`test_enrichment.py`, 16), artifact build (`test_historical_builder.py`, 7), reader parity (`test_historical_parity.py`, 3), the invariants on primitives (`test_input_integrity_e2e.py`, 8), the wire end to end (`test_hop_b_sink_e2e.py`, 6), the null-gate (`test_validation.py`, 14). What remains uncovered is **the manager orchestrating them**, which needs views-pipeline-core and a production-like Appwrite environment — and þing-02 **D2** forbids integration tests against the production project, no non-production one existing. + +That gap has **two standing trackers already**, which is why keeping a third here is noise rather than signal: issue **#18** (open since 2026-06-04) and `tests/test_falsification_campaign_3_5.py`, an `xfail(strict)` probe that **flips to XPASS the moment someone writes the test** — a self-surfacing tracker, which is more than this entry was doing. | +| Tier | 3 | +| Source | `repo-assimilation` (2026-06-02), `test-review` (2026-06-02) | +| Trigger | When modifying the manager's `_validate()` or the enricher, verify that the test suite covers the changed behavior — end-to-end coverage across the enrich→validate path is still absent | +| Location | `tests/test_validation.py`, `views_postprocessing/unfao/managers/unfao.py` | + +Initial state was zero test coverage. A 73-test suite was written (2026-06-02) covering the (now-deleted) mapper's core guarantees and the validation logic (missing columns, null rejection, error messages). Remaining gaps after the mapper removal: (1) the validation tests replicate `_validate()` logic in a standalone function because `views-pipeline-core` is unavailable in test environments — if the real `_validate()` diverges, tests pass while production fails; (2) no end-to-end test enriches through `GaulLookupEnricher` then validates through the manager. + +Tier recalibrated from 2 to 3 during review-rr (2026-06-02): the gap is maintainability (test-code divergence, missing integration path), not structural fragility. + +**Update 2026-06-24:** narrowed with the mapper deletion (C-39, PR #42). The mapper-coverage dimension is gone with the mapper (`tests/test_mapping.py` deleted; the determinism/cache/shapefile/`ThreadPoolExecutor` gaps no longer exist). Two manager-side gaps remain: the standalone `_validate()` replica and the missing enrich→validate end-to-end test. + +--- + +--- + ### C-43: ADR-011 enrichment swap shipped without its output-equivalence proof — and the proof is now unrecoverable — RESOLVED | Field | Value | @@ -760,7 +773,7 @@ This is a textbook instance of **C-42**'s registered hazard (acting on a mis-sta **Residual (why this entry stays open):** the verification was a one-off session result, not a standing guarantee. Nothing in CI re-runs it, so a future lookup rebuild against a wrong or stale datafactory would ship silently exactly as before. **C-43 closes when `tests/test_gaul_lookup_fidelity.py` is committed and green** — the entry should then cite the test, not the session. Tracked as **Cluster K**; the same test discharges C-59 and C-61. -See also C-03 (the sibling enrich→validate test-coverage gap), C-22 (no post-delivery correction/recall process — **now acute: the consequence path is live**), C-39 / C-31 / C-23 (the resolved mapper-deletion cluster this emerged from), C-30 (coverage — discharged by the same run that left this standing), C-32 / C-34 (RESOLVED — the go-global scale risks that fired cleanly), D-08 (the swap-to-lookup-first decision whose verification debt this is), #131 (run-0 delivery-integrity verification). +See also C-03 (the sibling enrich→validate test-coverage gap — RESOLVED 2026-08-02; its manager-orchestration residual relocated to #18), C-22 (no post-delivery correction/recall process — **now acute: the consequence path is live**), C-39 / C-31 / C-23 (the resolved mapper-deletion cluster this emerged from), C-30 (coverage — discharged by the same run that left this standing), C-32 / C-34 (RESOLVED — the go-global scale risks that fired cleanly), D-08 (the swap-to-lookup-first decision whose verification debt this is), #131 (run-0 delivery-integrity verification). --- diff --git a/tests/test_falsification_campaign_3_2.py b/tests/test_falsification_campaign_3_2.py index 0014fa7..b13f85a 100644 --- a/tests/test_falsification_campaign_3_2.py +++ b/tests/test_falsification_campaign_3_2.py @@ -4,6 +4,16 @@ Generated by falsification campaign on 2026-06-02 READ-ONLY CAMPAIGN: These stubs document test coverage gaps. + +DISCHARGED 2026-08-02 (S4 / #185): `test_falsify_3_2_validation_uses_replica_not_real_code` +is removed — the replica it described is gone. `tests/test_validation.py` now tests +`contract/historical.assert_metadata_complete` directly, parametrised over the imported +`gaul_schema.METADATA_COLS`. Mutation-proven: narrowing the gate to one column fails 9 of +its 14 tests, where the replica-based suite passed that mutation untouched. + +A probe written as `assert False` can never flip to XPASS, so the "surfaced for promotion" +mechanism below cannot reach it — a discharged probe has to be retired by hand, and leaving +one in place asserts a defect that no longer exists. """ import pytest @@ -25,16 +35,3 @@ def test_falsify_3_2_wrong_type_columns_not_tested(): """ assert False, "No test for wrong-type column values (e.g., string in numeric column)" - -def test_falsify_3_2_validation_uses_replica_not_real_code(): - """ - test_validation.py uses a standalone validate_dataframe() function that - replicates the logic from UNFAOPostProcessorManager._validate(). If the - real _validate() changes (e.g., adds a new column to the required list), - the replica won't be updated and tests will pass while production fails. - - This is a structural test-code coupling gap, not a missing test case. - - Severity: Soft falsification - """ - assert False, "Validation tests use replicated logic, not the real _validate() method — divergence risk" diff --git a/tests/test_falsification_campaign_4_3.py b/tests/test_falsification_campaign_4_3.py index 7902984..13638a7 100644 --- a/tests/test_falsification_campaign_4_3.py +++ b/tests/test_falsification_campaign_4_3.py @@ -2,6 +2,23 @@ Falsification test stubs for Campaign Claim 4.3: "Schema change would fail a test" Generated by falsification campaign on 2026-06-02 + +DISCHARGED 2026-08-02 (S4 / #185): `test_falsify_4_3_filter_cols_change_not_detected_by_validation_tests` +is removed. The hardcoded `REQUIRED_METADATA_COLS` replica it described is gone; +`tests/test_validation.py` parametrises over the imported `gaul_schema.METADATA_COLS`, so +adding a column to the declared contract extends the gate's coverage automatically — the +property register C-70 was resolved to give us. + +`test_falsify_4_3_enrichment_test_checks_only_5_of_9_filter_cols` is removed for the same +reason, verified in the same pass. It described `test_enrichment` asserting 5 of the 9 +contract columns. That file now imports `METADATA_COLS` and asserts +`list(lookup.columns) == METADATA_COLS` — exact equality on all nine — then iterates the +full set again for presence and for nulls. The four columns the probe said had "no schema +drift protection" are covered three times over. + +**Claim 4.3 is therefore fully discharged and this module holds no live probe.** It is kept +rather than deleted because the campaign record is the point: it says what was checked on +2026-06-02, what was wrong, and when each finding was closed. """ import pytest @@ -12,28 +29,3 @@ pytestmark = pytest.mark.xfail(reason="falsification probe — asserts a known-open finding by design (see reports/technical_risk_register.md, C-36); xfail(strict) keeps the suite green-when-healthy and surfaces any probe that starts passing", strict=True) -def test_falsify_4_3_enrichment_test_checks_only_5_of_9_filter_cols(): - """ - test_enrichment_adds_expected_columns checks 5 columns: - country_iso_a3, admin1_gaul1_code, admin1_gaul1_name, - admin2_gaul2_code, admin2_gaul2_name. - - But filter_cols in unfao.py has 9 columns (also: pg_xcoord, pg_ycoord, - admin1_gaul0_code, admin1_gaul0_name). Removing the missing 4 from - either side would not fail any test. - - Severity: Soft falsification - """ - assert False, "Enrichment schema test checks 5 of 9 filter_cols — 4 columns have no schema drift protection" - - -def test_falsify_4_3_filter_cols_change_not_detected_by_validation_tests(): - """ - The validation tests use a hardcoded REQUIRED_METADATA_COLS list that - replicates unfao.py's _necessary_metadata_cols. If someone adds or - removes a column from unfao.py's list, the test replica is not - automatically updated — the tests continue to pass against the old list. - - Severity: Soft falsification - """ - assert False, "Validation test's column list is a replica — adding/removing a column in unfao.py is not caught" diff --git a/tests/test_validation.py b/tests/test_validation.py index df791a8..93f0243 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -1,147 +1,151 @@ -"""Tests for the metadata validation logic in UNFAOPostProcessorManager._validate(). +"""The metadata null-gate, tested where it actually lives (S4 / #185, register C-03). -Covers the C-01 fix (null validation re-enabled) and CIC failure modes. -See docs/CICs/UNFAOPostProcessorManager.md for the contract under test. +**What this file used to be, and why it had to change.** It opened with: -Note: views-pipeline-core may not be installed in test environments, so we -replicate the validation logic here rather than importing the manager class. -The logic tested matches unfao.py:_validate() exactly. + Note: views-pipeline-core may not be installed in test environments, so we + replicate the validation logic here rather than importing the manager class. + The logic tested matches unfao.py:_validate() exactly. + +Both halves stopped being true. + +*The replica.* The file defined its own ``validate_dataframe`` and then tested that. +Forty-odd parametrised cases exercised a function declared twelve lines above them and +**no production code at all** — a closed loop that could not fail while the delivery +broke, and could not tell a reader anything about the system. + +*The fidelity claim.* ``_validate`` stopped null-gating in #149. Its own docstring now +says so: *"Neither payload is null-gated here."* It asserts the read **resolved** and +calls ``_check_coverage``. The null gate moved to ``contract/historical`` and fires at +artifact-build time. So the file asserted equivalence with a method it no longer +resembled, and had done since 2026-07-31. + +*The column list.* ``REQUIRED_METADATA_COLS`` was a nine-element literal — a hand copy +of the contract that register **C-70** was resolved to make single-source. It is now +imported. + +**What this file is now.** Tests of ``historical.assert_metadata_complete`` — the code +that actually gates a delivery — plus source-scan pins that the gate stays at build time +rather than drifting back into ``_validate``. The original justification for the replica +no longer applies either: ``contract/`` is dependency-light and imports without +views-pipeline-core (``tests/test_clone_readiness.py`` proves it in a subprocess). + +The behaviour C-01 fixed — a null in a required GAUL column must stop the delivery +rather than ship — is preserved in full. It is simply asserted against the function that +performs it. """ -import pytest -import pandas as pd +from __future__ import annotations + +from pathlib import Path + import numpy as np -import logging +import pyarrow as pa +import pytest + +from views_postprocessing.contract import historical +from views_postprocessing.contract.gaul_schema import CODE_COLS, COORD_COLS, METADATA_COLS + +_MANAGER_SOURCE = ( + Path(__file__).resolve().parent.parent + / "views_postprocessing" / "unfao" / "managers" / "unfao.py" +) +_HISTORICAL_SOURCE = ( + Path(__file__).resolve().parent.parent + / "views_postprocessing" / "contract" / "historical.py" +) + +def _artifact(n_rows: int = 5) -> pa.Table: + """A minimal historical artifact shaped like ``build_historical_table``'s output. + + Columns and dtypes come from ``gaul_schema`` (C-70), so this helper cannot drift + from the declared contract — a rename there breaks the import, not a later assert. + """ + columns = { + "month_id": pa.array(np.full(n_rows, 500, dtype=np.int64)), + "priogrid_id": pa.array(np.arange(100_001, 100_001 + n_rows, dtype=np.int64)), + "sb_best": pa.array(np.zeros(n_rows, dtype=np.float32)), + } + for col in METADATA_COLS: + if col in CODE_COLS or col in COORD_COLS: + columns[col] = pa.array(np.arange(n_rows, dtype=np.float64)) + else: + columns[col] = pa.array([f"{col}-value"] * n_rows, type=pa.string()) + return pa.table(columns) + + +def _with_null(col: str, row: int = 0, n_rows: int = 5) -> pa.Table: + """The artifact with one metadata value knocked out — the C-01 defect, reproduced.""" + table = _artifact(n_rows) + values = table.column(col).to_pylist() + values[row] = None + idx = table.column_names.index(col) + return table.set_column(idx, col, pa.array(values, type=table.column(col).type)) -logger = logging.getLogger(__name__) +# ── the gate ──────────────────────────────────────────────────────────────── -REQUIRED_METADATA_COLS = [ - "pg_xcoord", "pg_ycoord", "country_iso_a3", - "admin1_gaul1_code", "admin1_gaul1_name", - "admin1_gaul0_code", "admin1_gaul0_name", - "admin2_gaul2_code", "admin2_gaul2_name", -] +def test_a_complete_artifact_passes(): + historical.assert_metadata_complete(_artifact()) # must not raise -def make_valid_dataframe(n_rows=5): - """Create a DataFrame that passes all validation checks.""" - return pd.DataFrame({ - "pg_xcoord": np.random.uniform(0, 50, n_rows), - "pg_ycoord": np.random.uniform(-10, 10, n_rows), - "country_iso_a3": ["AAA"] * n_rows, - "admin1_gaul1_code": [101] * n_rows, - "admin1_gaul1_name": ["A-South"] * n_rows, - "admin1_gaul0_code": [1] * n_rows, - "admin1_gaul0_name": ["CountryA"] * n_rows, - "admin2_gaul2_code": [1021] * n_rows, - "admin2_gaul2_name": ["A-South-W"] * n_rows, - "pred_value": np.random.uniform(0, 1, n_rows), - }) +@pytest.mark.parametrize("col", METADATA_COLS) +def test_a_null_in_any_required_metadata_column_stops_the_delivery(col): + """C-01: incomplete geographic metadata must never reach the UN FAO. + + Parametrised over the **imported** contract, not a copy — add a column to + ``gaul_schema.COLUMNS`` and it is gated here automatically, which is the property + C-70 was resolved to give us. + """ + with pytest.raises(historical.HistoricalArtifactError): + historical.assert_metadata_complete(_with_null(col)) + + +def test_the_refusal_names_the_column_and_counts_the_nulls(): + """A refusal an operator cannot act on is barely better than no refusal.""" + with pytest.raises(historical.HistoricalArtifactError) as excinfo: + historical.assert_metadata_complete(_with_null("country_iso_a3", n_rows=10)) + message = str(excinfo.value) + assert "country_iso_a3" in message, "the refusal must name the offending column" + assert "1/10" in message, "the refusal must say how much of the artifact is affected" + + +def test_every_declared_metadata_column_is_gated(): + """The gate iterates the declared contract; nothing is gated by coincidence. + + Reads the source on purpose — the parametrised test above proves each column *is* + caught; this proves it is caught **because** the column is declared, rather than + because someone happened to list it twice. + """ + assert "for col in METADATA_COLS:" in _HISTORICAL_SOURCE.read_text(), ( + "assert_metadata_complete must iterate the declared METADATA_COLS. A hand-written " + "column list here is register C-70 returning: one contract, two spellings." + ) + +# ── where the gate lives, and where it must not drift back to ─────────────── -def validate_dataframe(df, label="dataframe"): - """Replicates the validation logic from UNFAOPostProcessorManager._validate(). +def test_the_manager_does_not_null_gate_in_validate(): + """#149 moved the gate to build time. A silent move back would double-gate the + historical leg while leaving the forecast leg — which is arrow, not pandas — with a + gate written for a representation it no longer uses. - This must stay in sync with views_postprocessing/unfao/managers/unfao.py. + Source-scan because the manager needs views-pipeline-core and Appwrite env to + instantiate; this is the repo's standing pattern for manager-side facts. """ - for col in REQUIRED_METADATA_COLS: - if col not in df.columns: - err_msg = f"{label} is missing required metadata column: {col}. Found columns: {df.columns.tolist()}" - logger.error(err_msg) - raise ValueError(err_msg) - null_count = df[col].isnull().sum() - if null_count > 0: - err_msg = f"{label} has {null_count} null values in required metadata column: {col} ({null_count}/{len(df)} rows)." - logger.error(err_msg) - raise ValueError(err_msg) - - -# --------------------------------------------------------------------------- -# GREEN: Valid data passes validation -# --------------------------------------------------------------------------- - -class TestValidDataPasses: - def test_valid_dataframes_pass(self): - validate_dataframe(make_valid_dataframe(), "Historical dataframe") - validate_dataframe(make_valid_dataframe(), "Forecast dataframe") - - def test_extra_columns_are_fine(self): - df = make_valid_dataframe() - df["extra_col"] = "foo" - validate_dataframe(df) - - -# --------------------------------------------------------------------------- -# RED: Missing columns raise ValueError (CIC §6) -# --------------------------------------------------------------------------- - -class TestMissingColumns: - @pytest.mark.parametrize("col", REQUIRED_METADATA_COLS) - def test_missing_historical_column_raises(self, col): - df = make_valid_dataframe().drop(columns=[col]) - with pytest.raises(ValueError, match="missing required metadata column"): - validate_dataframe(df, "Historical dataframe") - - @pytest.mark.parametrize("col", REQUIRED_METADATA_COLS) - def test_missing_forecast_column_raises(self, col): - df = make_valid_dataframe().drop(columns=[col]) - with pytest.raises(ValueError, match="missing required metadata column"): - validate_dataframe(df, "Forecast dataframe") - - -# --------------------------------------------------------------------------- -# RED: Null values raise ValueError (C-01 fix) -# --------------------------------------------------------------------------- - -class TestNullValuesRejected: - @pytest.mark.parametrize("col", REQUIRED_METADATA_COLS) - def test_null_in_historical_column_raises(self, col): - df = make_valid_dataframe() - df.loc[0, col] = None - with pytest.raises(ValueError, match="null values"): - validate_dataframe(df, "Historical dataframe") - - @pytest.mark.parametrize("col", REQUIRED_METADATA_COLS) - def test_null_in_forecast_column_raises(self, col): - df = make_valid_dataframe() - df.loc[0, col] = None - with pytest.raises(ValueError, match="null values"): - validate_dataframe(df, "Forecast dataframe") - - def test_all_null_column_raises(self): - df = make_valid_dataframe() - df["country_iso_a3"] = None - with pytest.raises(ValueError, match="null values"): - validate_dataframe(df) - - def test_nan_treated_as_null(self): - df = make_valid_dataframe() - df.loc[0, "pg_xcoord"] = float("nan") - with pytest.raises(ValueError, match="null values"): - validate_dataframe(df) - - -# --------------------------------------------------------------------------- -# RED: Error messages include useful context (ADR-008) -# --------------------------------------------------------------------------- - -class TestErrorMessages: - def test_missing_col_error_names_the_column(self): - df = make_valid_dataframe().drop(columns=["country_iso_a3"]) - with pytest.raises(ValueError, match="country_iso_a3"): - validate_dataframe(df) - - def test_null_error_includes_count(self): - df = make_valid_dataframe(n_rows=10) - df.loc[0:2, "pg_xcoord"] = None # 3 nulls - with pytest.raises(ValueError, match="3"): - validate_dataframe(df) - - def test_errors_are_logged(self, caplog): - df = make_valid_dataframe().drop(columns=["country_iso_a3"]) - with caplog.at_level(logging.ERROR): - with pytest.raises(ValueError): - validate_dataframe(df) - assert any("country_iso_a3" in r.message for r in caplog.records) + source = _MANAGER_SOURCE.read_text() + validate_body = source.split("def _validate(")[1].split("def _check_coverage(")[0] + for token in ("isnull", "is_null", "null_count", "METADATA_COLS"): + assert token not in validate_body, ( + f"_validate references {token!r} again. The metadata null-gate belongs at " + "artifact build (historical.assert_metadata_complete), where the artifact " + "exists; _validate asserts that the read resolved. See #149." + ) + + +def test_the_manager_still_calls_the_gate_on_the_built_artifact(): + """The other half: having moved, the gate must actually be invoked somewhere.""" + assert "historical.assert_metadata_complete(" in _MANAGER_SOURCE.read_text(), ( + "nothing calls the metadata null-gate — the historical artifact would ship with " + "missing geography and no error signal (register C-01, C-43)." + )