Repository navigation
feat(delivery): FAO input-integrity invariants S0–S6 (#51) - #64
Merged
Merged
Conversation
…d-range guards (#51) Introduces the representation-free delivery-integrity layer and wires the first two guards into the FAO manager (epic #51, S0-S2). - views_postprocessing/delivery/: partner-agnostic, pandas-free invariants — coverage (S1/C-34) and observed_range (S2/C-26). Called by managers, never inherited. - unfao/extraction.py: the single pandas-aware seam feeding primitives to the invariants (swappable when the delivery representation leaves pandas, C-40). - unfao/source_metadata.py: producer data-facts read straight from views-datafactory (last_valid_month_id), never via pipeline-core (D-07). - unfao manager: _clip_observed_history (drop fabricated tail months > last_valid_month_id, historical frame only) and _check_coverage (enforce pinned-region cell counts, else log-skip). - enrichment.py: "prediction frame" -> "input DataFrame" naming (frame conflation). - unit + design-contract tests for each. S3-S6 (#54-#57) remain. Reconciliation/ still present here (predates PR #63; deferred sync). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…curation Accumulated risk-register edits that rode this branch: - Reconciliation migration end-state (verified on origin/development): C-42 corrected (the false "vpp unused" claim; then release v1.7.0 -> repoint views-models#191/PR#202 -> delete #62/PR#63; pipeline-core chose Decision K, not C; mis-stated-state hazard resolved; residual ForecastReconciler coupling tracked by pipeline-core#198, no separate entry). C-37/C-38 relocate to views-frames with the code; C-38 dead-link to the deleted CIC repaired. - D-07 resolved: data-facts come from the producer (datafactory/viewser), not pipeline-core. - /review-rr curation + input-integrity entries (C-26, C-34). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…valence proof (Tier 2) The ADR-011 enrichment swap (runtime mapper -> GAUL lookup enricher) went live without the output-equivalence proof its own plan required (umbrella #20: Stage 0 baseline + Stage 2 diff, "zero unexplained differences"). That proof was never produced, and the old mapper + both diff scripts were since deleted (eba1df8, PR #42), so it is now unrecoverable. Accepting option A (a smoke-test delivery) verifies the path runs, not that it produces the same/correct values; _validate only checks the 9 columns are non-null, not correct, so a lookup/merge bug ships wrong-but-non-null geo metadata to FAO silently. Tier 2 (not 1: the lookup sources from datafactory's authoritative GAUL parquets, D-07); trigger is the go-global flip to land_gaul (64k cells). Header 42/22/20 -> 43/23/20. Cross-refs C-03, C-22, C-23, C-30/32/34, C-39, D-08. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… guard (#54, C-25) The FAO forecast is selected from the prediction store by category alone (newest-wins) with no filter on ensemble name or loa, so a stray category="forecast" upload (a test run, a second model, a backfill) would be silently enriched and shipped to FAO (C-25). S3 adds a representation-free guard that fails loud when the selected file's identity does not match the configured ensemble. Mirrors the S0-S2 pattern exactly: a primitives-only invariant in delivery/ (assert_forecast_identity), fed by the extraction seam (file_metadata normalizes the datastore metadata record to a plain dict), called by the manager. _read_forecast_data now resolves the file id, asserts identity ({name: model_name, loa: "pgm"}) before download, instead of the single download_latest_file convenience call. Residual tracked in C-25: the producer's uploaded name/loa contract is verified against pipeline-core's code but not a live upload — confirmed at the first real run; the guard fails loud (not silent) if the contract is off. Tests: unit on dicts (match/mismatch/missing-key/label) + extraction normalization. ruff clean; 101 passed, 44 xfailed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nifest (#55, C-30) At the land_gaul switch, the GAUL-uncovered sub-Antarctic cells have no country assignment — shipping them attributes FAO rows to a non-country (-1), or crashes on delivery day (C-30, Tier 1). S4 pins the excluded cells as an explicit frozen manifest and fails loud if any reach the delivery. Mirrors S1-S3: a representation-free manifest + check in delivery/coverage.py (EXCLUDED_GIDS_BY_REGION, excluded_for, assert_no_excluded_cells), called region-gated by the manager's _check_coverage (a no-op for unpinned africa_me_legacy, so its 5 ocean cells are unaffected). The 76 are disclosed in docs/fao_excluded_cells.md. Count correction (the frozen-list tripwire working as designed): deriving from the live producer (datafactory v1.4.0) gives 64,742 complete + 76 excluded, not the 64,736 + 82 the register/issue carried. Cause: datafactory #163 (ADR-043) supplemented 6 Azorean cells into land_gaul (82 - 6 = 76). vpp's own built lookup already ships 64,742, so the stale pin would have false-positived against our own artifact. Authoritative source = land - land_gaul (76), not raw gaul0_code == -1 (82). EXPECTED_CELLS pin corrected; C-30 + #55 updated. Tests (8 new): exclusion count == 76; known sub-Antarctic gids excluded; Azorean cells NOT excluded; leaked cell raises naming the gid; unpinned region no-op; plus a drift tripwire cross-checking the manifest against the datafactory sibling when present. ruff clean; 109 passed, 44 xfailed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ovenance (#56, C-15) Upload metadata carried only a free-text description, so FAO could not audit what produced their data (C-15). S5 stamps each upload with structured, sourced provenance. Mirrors S1-S4: a representation-free delivery/provenance.py (build_provenance -> dict) assembling lookup_version, region, expected/actual cell counts, and unmapped_count from the enricher + S1 coverage + a new extraction.unmapped_cell_count seam — nothing hardcoded. The manager's _delivery_description builds it per file and both _save uploads carry it. Carrier constraint (surfaced): pipeline-core's upload_data exposes no structured field — only free-text description — so the dict is JSON-encoded into description behind a human prefix for now. A dedicated metadata field is requested upstream (pipeline-core #245); when it lands, only the manager's attach step changes (the provenance shape is already representation-free). fill_count omitted until a fabricated-value count exists (cf. C-26). Tests (9 new): provenance dict fields/optionality/JSON-serializability; unmapped_cell_count on mapped/null/missing-col frames. ruff clean; 117 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gh the manager seam (#57) Proves all five guards fire end-to-end on synthetic delivery frames. The manager can't be instantiated (pipeline-core + Appwrite env, C-40), so — like test_append_metadata.py / test_validation.py — each test replicates the manager's own extraction->invariant chain and asserts the guard: S1 coverage cells_of -> assert_complete_coverage (correct passes; under/over raise) S2 observed range months_of -> fabricated_months -> drop_months_above (month > boundary not shipped as zero) S3 forecast identity file_metadata -> assert_forecast_identity (decoy rejected; match passes) S4 land_gaul cells_of -> assert_no_excluded_cells (injected 62356 crashes loud) S5 provenance build_provenance <- cells_of/unmapped_cell_count (description carries structured fields) S4 uses the corrected 64,742 + 76 manifest (not the issue's stale 64,736 + 82). The design-contract module is now 5 passed / 1 xfail — the only remaining input-integrity xfail is the C-40-deferred manager de-inheritance. ruff clean; 126 passed, 44 xfailed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#64) - _clip_observed_history: degrade open consistently. The boundary fetch returned None when the attribute is absent (skip clip, warn) but RAISED on a transient datafactory network error (URLError/HTTPError/TimeoutError), which propagated through _read_historical_data (no try/except) and crashed the whole delivery. Wrap the fetch so both unresolved cases are treated alike. - CIC: document the four new fail-loud guards (ForecastIdentityError, CoverageError on coverage/exclusion, observed-range clip, structured provenance) in §6, and the new invariant + e2e test modules in §10 — the contract doc had drifted from the manager's now-richer loudness contract. - extraction.unmapped_cell_count: type the metadata_cols param (Sequence[str]). ruff clean; 126 passed, 44 xfailed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 26, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the FAO delivery input-integrity epic (#51): six representation-free guards that fail loud instead of shipping silent bad data to the partner.
Design
Each guard is a representation-free invariant in
views_postprocessing/delivery/(primitives only — no pandas), fed by theunfao/extraction.pypandas→primitives seam, and called by the manager (never inherited). This survives the eventual pandas→views-frames migration untouched (only the seam changes; C-40), and keeps the C-40 manager de-inheritance cleanly deferred.Guards
last_valid_month_idcategory=forecastuploadCorrections surfaced during build
64,736 + 82was stale; the live producer (datafactory v1.4.0) is 64,742 complete + 76 excluded (datafactory refactor(structure): S5 — separate partner declarations from delivery machinery #163 / ADR-043 supplemented 6 Azorean cells intoland_gaul). vpp's own lookup already shipped 64,742, so the old pin would have false-positived against our own artifact. A drift tripwire test now cross-checks the manifest against the datafactory sibling.upload_datahas no structured-metadata field, so provenance is JSON-encoded intodescriptionfor now; a dedicated field is requested at pipeline-core S4: delete the unfiltered arrival check; keep the row differential #245.Residuals (tracked, not blockers)
Verification
ruffclean; 126 passed, 44 xfailed. The design-contract module is 5 passed / 1 xfail (the C-40-deferred inheritance contract).Risk register updated throughout (C-15, C-25, C-30, C-43 + new C-43 entry). Story issues close on merge.
🤖 Generated with Claude Code