diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..c0aeb8e --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,82 @@ +# Changelog + +What changed **for a consumer** — the launcher that pins this package, and the partner +that receives its artifacts. Not a commit log: entries here name behaviour someone +outside this repository can observe, in particular **behaviour that can turn a +previously-passing run into a failing one**. + +This file exists because the version number was the only signal a consumer got +(register C-111). Releases before 1.2.0 are summarised from their tags rather than +reconstructed in detail. + +## 1.2.0 — 2026-08-26 + +**A previously-passing delivery can now fail in three new ways. All three are +deliberate, and each replaces a silent failure with a loud one.** + +### New failure modes that escape into the launcher + +- **`views_postprocessing.delivery.findability.DeliveryNotFindableError`** — after + upload, the delivery now asks the store the same question the consumer asks: *is + the newest document under the consumer's name the one this run just uploaded?* If + it is not, the run raises. **Previously a delivery whose artifacts landed somewhere + the consumer could not see reported success.** (C-94) + +- **`…findability.FindabilityUnverifiedError`** — raised when the check itself could + not run, e.g. the store errored on the read-back. Deliberately distinct from + `DeliveryNotFindableError`: "could not ask" is not "asked and got nothing". + +- **`views_postprocessing.contract.source_metadata.ProducerClientUnavailable`** — + reading the producer's `last_valid_month_id` now raises if `datafactory_query` + cannot be loaded, whether it is absent or raises on import. **Previously a broken + environment degraded open and shipped unobserved months as observed history.** + A producer that publishes no boundary is still handled as before — that is a normal + older store, and a different condition. (C-103) + +- **`views_postprocessing.contract.wire.sink.TornRunError`** — a failure partway + through uploading a run now raises a refusal naming the run, every object confirmed + uploaded, and the object that failed. It does **not** delete anything; a torn run + still leaves orphans, but it no longer leaves them undocumented. Note this is a + `RuntimeError`, not a `SinkError` — `SinkError` means do-not-retry, and a torn run + may be retried. (C-105) + +### Changed data reaching the partner + +- **Delivered artifacts carry a new provenance field, `observed_through`.** It records + the producer boundary the observed-range clip used, or `null` when the boundary + could not be read and the clip was therefore **skipped** — meaning unobserved months + may be present. Carried in the file's `description` metadata, which is a JSON string; + consumers that treat that field as opaque text are unaffected. (#297) + +### Changed internals a direct caller would notice + +- `unfao.store_port` / `crafd.store_port` `upload()` now **returns the uploaded file + id** instead of discarding it. Required by the findability read-back above. +- `delivery.provenance.build_provenance()` gained a **required** keyword argument, + `observed_through`. Required rather than optional because a caller that omits it is + the failure being fixed. + +### Documentation + +- ADR-013 gains **§5.1a**, recording that nullable int64 GAUL code columns were + considered and rejected, with the measured pandas round-trip behaviour that decided + it. No `contract_version` change: the wire bytes are untouched. (#278) + +### Upgrading + +Nothing to change in a launcher. The three new exceptions surface conditions that were +already wrong and already silent — if one fires after upgrading, it is reporting a +pre-existing problem, not creating a new one. + +## 1.1.1 — 2026-08-15 + +Fixes C-99. Consumers on 1.1.0 should move; see views-models#403. + +## 1.1.0 — 2026-08-13 + +Adds the CRAF'd producer package alongside UN-FAO, and the observed-range clip that +drops months above the producer's declared boundary. + +## 1.0.0 — 2026-08-01 + +First released version. diff --git a/pyproject.toml b/pyproject.toml index f42e9a5..d31e13d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "views-postprocessing" -version = "1.1.1" +version = "1.2.0" description = "" authors = [ "Dylan Pinheiro ", diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index 2dda7c5..b1d07e5 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -4,10 +4,10 @@ |-------------------|--------------------------------------| | Project | views-postprocessing | | Owner | Dylan Pinheiro / PRIO MD&D Team | -| Last Updated | 2026-08-21 | -| Total Concerns | 109 | -| Open Concerns | 29 | -| Resolved Concerns | 80 | +| Last Updated | 2026-08-26 | +| Total Concerns | 112 | +| Open Concerns | 31 | +| Resolved Concerns | 81 | --- @@ -52,9 +52,15 @@ covered a single open entry (see Historical clusters below). **Fix strategy:** this repo already solved this disease once — the ADR-013 audit series ended with a **permanent guard suite** (`tests/test_falsify_adr013_*.py` — `pytest --collect-only -q tests/test_falsify_adr013*.py` for the count, which moves), and the same pattern now guards the þing-01 invariants (`tests/test_env_declaration.py`, `tests/test_redaction_guard.py` — the latter briefly **only over the roots that still existed**, see C-74, resolved: a guard is only as good as the assertion that its inputs are real, and it now carries that assertion). There is **no equivalent for the register**. A small `tests/test_register_integrity.py` — header counts match section counts; no RESOLVED body under `## Open Concerns`; every `C-\d+`/`D-\d+` reference resolves or is namespaced to a foreign register — would make this class self-detecting. **Resolution scope:** Full for the mechanical half. +**Succeeded by Cluster N.** This cluster's disease was governance prose drifting out of step with cross-repo state; its mechanical half is guarded now. What replaced it is broader and is tracked as **Cluster N, "This repository cannot see itself"** — the same surface, including the parts no guard reaches. + +**✅ CLOSED 2026-08-21 (review-rr triage).** Every entry cited above is resolved, and the cluster's own prescription landed: it asked for *"a small `tests/test_register_integrity.py` — header counts match section counts; no RESOLVED body under `## Open Concerns`; every `C-\d+`/`D-\d+` reference resolves or is namespaced"*. That file exists and does exactly those three things, plus five more; it is green, and it caught two real errors during this week's registrations (a header count off by one, and a views-pipeline-core `C-241` written without its namespace). + +**The residual, and it is this cluster's disease one level up.** The guard covers entries. It does **not** cover the Causal Clusters section — which is how this cluster sat with nine resolved entries and no closure marker until a triage read it by hand, and how Cluster J went on calling a resolved C-22 *"acute"*. **C-109** records the same shape for `Location` line numbers. Nothing is proposed here: the honest position is that the mechanical half is done and the prose half is checked by reading, which is what triage is for. + ### Cluster J: Delivery aftercare has no mechanism **Root cause:** the delivery pipeline is write-only — nothing exists downstream of upload for correction, recall, or provenance audit. -**Entries:** C-22 (acute), C-15, C-24, C-105 (added 2026-08-16 — a torn upload attempt is aftercare the write-only path has no answer for) +**Entries:** C-15, C-24, C-105 (added 2026-08-16 — a torn upload attempt is aftercare the write-only path has no answer for) — plus **C-22, RESOLVED**, which this line called *"acute"* until 2026-08-21 **Highest tier:** 3 **Fix strategy:** the C-22 correction procedure (issue #15) plus pipeline-core #245's structured metadata field to retire the description-as-carrier abuse. **Resolution scope:** Partial (process, not code). @@ -159,6 +165,15 @@ that indexes only deleted code is noise. --- +### Cluster N: This repository cannot see itself +**Root cause:** the guards here are unusually good at checking code against a declaration, and absent wherever the subject is the repository's own prose, its own records, or its relationship to anything outside. Every entry below was filed separately; together they are one class. +**Entries:** C-109 (`Location` line numbers rot faster than the review cycle), C-107 (docstrings sit outside the doc-accuracy scan), C-95 (a verdict mis-cited in three places), C-110 (a release block this repo scheduled for itself, documented nowhere a releaser looks), C-111 (a release changes delivery behaviour and only the version number says so), C-112 (nothing observes what consumers actually run) — plus **C-81**, whose own headline number was falsified by PR #280 and stood stale for four days. +**Highest tier:** 2 (C-81) +**Fix strategy:** **not more guards.** C-109 records why the obvious mechanical check fails — content assertions need a second declaration that can itself go stale, which is ADR-014 §2's warning. What works is a *reading* pass: this cluster was assembled by `/review-rr strategic` on 2026-08-21, and four of its seven members were found by reading rather than by any test. Schedule the reading; do not build a linter for prose. +**Resolution scope:** Partial and by nature. C-107 and C-109 are closable as conventions (scan the docstrings; cite by symbol). C-111 and C-112 are closable as artifacts (a changelog; a pin check). C-95, C-110 and C-81 are closable only by someone re-reading what the register says and comparing it to what is true — which is the activity, not a deliverable. + +**Why this cluster was late.** Six of its seven entries arrived between 2026-08-16 and 2026-08-21, during one sprint, each filed as an unrelated finding. The register had no cluster for them because the clusters describe *delivery* risks — inherited surface, go-global debt, aftercare, the lookup artifact. Nothing described the governance layer as a risk surface of its own, even though the register is the artifact four repositories read. + ## Open Concerns ### C-98: A tripwire on another repo's release history watched our own pin, and reported two days late @@ -631,6 +646,8 @@ and silent about the *cost*. Adopting `[edition].obliges_consumers` is the defer would fix it, and its trigger is below. +**A second source of redness, and it is ours — see C-110.** From 2026-10-18 `tests/test_credential_expiry.py` fails by design, so this entry's no-bypass finding starts applying to a block this repository scheduled for itself rather than one a sibling caused. + --- ### C-87: The delivery label will be checked against a declaration, and nothing will check the declaration against the consumer @@ -720,6 +737,8 @@ Four companion tests keep it honest rather than decorative: the firing branch is Cross-refs: **C-81** (the same operator session's other half — branch protection and the CI token), **C-27** (no rotation mechanism for a secret value upstream), **C-57** (the pinned-registry detector, which is how this arrived here at all — it demanded the v1.4.4 bump and the bump is what surfaced the expiry), þing-02 A3(i), views-appwrite C-65 and C-66. +**The tripwire gates the release path, which this entry did not say when it was added — see C-110.** From 2026-10-18 it reddens `test`, which `protect_main` makes a required check with zero bypass actors, so no release can be tagged until the keys rotate or `ACKNOWLEDGED_UNTIL` is set. + --- ### C-15: Upload metadata lacks enrichment provenance and carries test description @@ -1017,17 +1036,28 @@ Cross-refs: **C-62** (the transitive dependency drag; the other 31 alerts), **C- --- -### C-81: What actually gates `main` is weaker than it looks — CI verifies 8 fewer tests than local; the enforcement half is discharged +### C-81: What actually gates `main` is weaker than it looks — CI verifies 2 fewer tests than local; the enforcement half is discharged | Field | Value | |-------|-------| | ID | C-81 | | Tier | 2 — the guards this arc built to catch cross-repo drift do not run where drift happens, and the branch they protect has no required check. Both halves are structural and both have fired-in-practice evidence. | | Source | `code-review max` (2026-08-03) — development→main sync audit | -| Trigger | **Coverage half, re-specified 2026-08-13:** the next time a views-datafactory change would break a delivery — its 8 gated tests are the whole remaining gap and none of them runs in CI. ~~*Original: when the Appwrite Seam Contract registry next moves, nothing in CI will notice.*~~ **That trigger is false** and has been since 2026-08-10: CI checks out views-appwrite at `ref: main` and sets `VIEWS_APPWRITE`, so every registry-drift detector runs there. ~~**Enforcement half:** the first time someone merges a red PR to `main`~~ — **DISCHARGED 2026-08-13**: `protect_main` now requires the `test` check (see C-86). | +| Trigger | **Coverage half, re-specified again 2026-08-21 — the previous wording is now false.** It read *"its 8 gated tests are the whole remaining gap and none of them runs in CI"*. PR #280 added the views-datafactory checkout, so four of those now run in the gate. The live trigger is what remains: **the next time a change to a views-datafactory artifact that is NOT in its git repository would break a delivery** — the producer-comparison half of `test_gaul_lookup_fidelity` and the two `test_datafactory_deploy_readiness` gates still skip in CI, because the GAUL parquets and `data/assembled/` are untracked upstream (C-46, C-108). ~~*Original: when the Appwrite Seam Contract registry next moves, nothing in CI will notice.*~~ **That trigger is false** and has been since 2026-08-10: CI checks out views-appwrite at `ref: main` and sets `VIEWS_APPWRITE`, so every registry-drift detector runs there. ~~**Enforcement half:** the first time someone merges a red PR to `main`~~ — **DISCHARGED 2026-08-13**: `protect_main` now requires the `test` check (see C-86). | | Owner | Simon — both halves need operator action. The coverage half needs a token for two private repositories; the enforcement half is a GitHub console/ruleset change. Neither is engineering work. | | Location | `.github/workflows/run_pytest.yml`; the `protect_main` ruleset; `tests/conftest.py::sibling_repo` | +**⚠ RE-MEASURED 2026-08-21: the headline number was 8 and is now 2, and this entry did not notice for four days.** + +| environment | result | +|---|---| +| CI-shaped (both siblings, tracked files only) | **468 passed, 5 skipped** | +| full local (both siblings complete) | **470 passed, 3 skipped** | + +PR #280 fetched views-datafactory in CI, moving four checks from skipped to running — C-30's exclusion-manifest tripwire, C-46's release gate, the region-set check and the wire-cast dtype check. The gap it measures closed by three-quarters and the entry went on stating the old figure, in the one place a reader goes to find out how strong the gate is. + +**That is not an aside.** This entry exists to measure the distance between what CI checks and what a laptop checks. Carrying a stale number is the same defect one level up, and it is why the "This repository cannot see itself" cluster names this entry alongside C-109 and C-107. + **Coverage.** Measured in an isolated clone, not estimated — **402 collected in every run**, so the whole delta is skips: | environment | result | @@ -1117,9 +1147,9 @@ Cross-refs: **C-26** (the fabrication this clip exists to prevent), **C-07** (un | ID | C-104 | | Tier | 3 — no production impact. The cost is that a red suite stops carrying signal, on precisely the two modules with the thinnest coverage. | | Source | `/repo-assimilation` (2026-08-16), measured | -| Trigger | When `pytest` reports failures in `tests/test_framework_contract.py` or `tests/test_store_construction.py`, check `pip show views-pipeline-core` against `poetry.lock` before reading them as defects. | +| Trigger | When `pytest` reports byte-parity or manager-import failures, run `tests/test_locked_environment.py` first and check the interpreter with `python -V` — on 3.12 or newer the drift **cannot** be reconciled and the remedy the test prints will fail. | | Owner | This repository. | -| Location | `tests/test_framework_contract.py`, `tests/test_store_construction.py` (20 failures); `tests/test_wire_shard.py`, `tests/test_wire_sidecar.py`, `tests/test_hop_b_sink_e2e.py` (5 failures); `poetry.lock` versus the project venv | +| Location | `tests/test_framework_contract.py`, `tests/test_store_construction.py` (20 failures); `tests/test_wire_shard.py`, `tests/test_wire_sidecar.py`, `tests/test_hop_b_sink_e2e.py` (5 failures); `poetry.lock` versus the project venv; `pyproject.toml` line 12 (`python = ">=3.11,<3.15"`) | Measured 2026-08-16 in the project venv: 458 collected, **433 passed, 25 failed**, 39 xfailed, in 14.85s. The venv holds `views-pipeline-core 2.3.0` and `pyarrow 23.0.1`; `poetry.lock` pins **3.0.1** and **16.1.0**. The pyarrow half is known and predicted: 5 byte-parity failures reporting *"pinned toolchain violated: byte-parity oracle requires pyarrow 16.1.0, found 23.0.1"*, exactly what `tests/fixtures/wire_contract/README.md` says will happen under **C-72**. The pipeline-core half is documented nowhere: `ModuleNotFoundError: No module named 'views_pipeline_core.modules.dataloaders.datafactory_contract'`, raised at import of both managers, which takes out every test that constructs or inspects one. @@ -1133,6 +1163,24 @@ It does **not** fix the drift and does not skip. The 25 failures remain until so Dev-group tools are deliberately out of scope: `ruff`'s reported version varies with how it was installed, and the thing that actually broke CI on 2026-08-03 was its *rule set*, which `pyproject.toml` already pins explicitly. +**2026-08-22 — the drift is not neglect: `poetry install` cannot succeed on this machine, and the entry's own remedy is the advice it warns about.** Running it took the suite from **26 failed / 470 passed** to **6 failed / 490 passed** — it upgraded `views-pipeline-core` 2.3.0 → 3.0.1 and then died building `levenshtein 0.20.9` from source. What remains is a single drifted package, `pyarrow` installed 23.0.1 against 16.1.0 locked, and all six failures are that one cause: five C-72 byte-parity checks plus the lock check reporting it. No defects among them. + +The build failure is not incidental. The developer venv is **Python 3.13.7**; wheel availability for the two blocking packages, measured directly against the index: + +| interpreter | `pyarrow 16.1.0` | `levenshtein 0.20.9` | +|---|---|---| +| cp311 | yes | yes | +| cp312 | yes | **no** | +| cp313 | **no** | **no** | + +`levenshtein` is capped `>=0.20,<0.21` by **`ingester3`**, and no release in that range publishes a 3.12+ wheel — so the cap, not this repository, is what fixes the ceiling. **`poetry.lock` is installable on cp311 alone.** CI runs 3.11 and the delivery's own conda prefix is 3.11.15 holding `pyarrow 16.1.0`; both match the lock exactly. Only the developer venv is off, and it cannot be brought back by the command everyone is told to run. + +That makes the mitigation above **half wrong in the way it names as a failure**. Its own text rules out reporting a package as "run `poetry install`" when that advice cannot work — and on a 3.12+ interpreter the drift line does exactly that, sending the reader at a command that will fail on a package they have never heard of, thrown by a transitive dependency of a dependency. The remedy is a machine action still, but a different one: rebuild the venv on 3.11, not reinstall on 3.13. + +The second half is that `pyproject.toml` declares `python = ">=3.11,<3.15"`, which the table above shows is **false** — a contributor arriving on 3.12 or 3.13 is told the project supports them and then cannot install it. Tracked separately as **#295**, because it is a declaration defect rather than an environment one and its fix is a one-line change with a cross-repo cause. + +**This entry stays open, and what closes it has changed.** It is no longer "someone runs `poetry install`" — that is now known not to work here. It is that the venv is rebuilt on a 3.11 interpreter, which is an operator action on the machine. + Cross-refs: **C-72** (owns the pyarrow half — that half is not re-registered here), **C-81** (CI-versus-local coverage asymmetry), **C-36** (a permanently-red suite cannot detect new regressions), **C-102** (the same argument in the other direction: a guard that never runs proves nothing, and a failure nobody can read is not a signal). --- @@ -1166,7 +1214,7 @@ Cross-refs: **C-94** (nothing observes the outcome of an upload at the time it h --- -### C-106: The §2 header builder — the module that owns the contract version — is reachable only from tests +### C-106: The §2 header builder — the module that owns the contract version — is reachable only from tests `[backlog]` | Field | Value | |-------|-------| @@ -1243,7 +1291,57 @@ C-103 is the sharp case, and the reason this is a class rather than three typos: **Why the obvious guard does not work, stated so it is not proposed again cheaply.** Checking that a file has at least that many lines catches nothing: every stale citation above points at a real line. Checking *content* requires the entry to declare what it expects to find there, which is a second declaration that can itself go stale — the shape ADR-014 §2 warns about. The cheap and durable move is the convention (`::symbol`), not a test. -Cross-refs: **C-103** (twice stale in four days — the case that made this visible), **C-107** (docstrings outside the doc-accuracy scan; the same "nothing checks the prose" family), **C-82** (governance prose carrying numbers nothing checks, resolved). +Cross-refs: **C-103** (twice stale in four days — the case that made this visible), **C-95** (a verdict mis-cited in three places — the same defect in prose rather than in a line number, and the nearest sibling), **C-107** (docstrings outside the doc-accuracy scan; the same "nothing checks the prose" family), **C-82** (governance prose carrying numbers nothing checks, resolved). + +--- + +### C-110: The release path will block itself from 2026-10-18, and nothing tells the person it blocks + +| Field | Value | +|-------|-------| +| ID | C-110 | +| Tier | 3 — the ability to ship is interrupted on a known date, but the escape is in-repo and reachable. That is what separates it from **C-86**'s Tier 2, where the only responses are console actions by one person. If the acknowledgement were ever removed, or the tripwire made unconditional, this becomes C-86's tier. | +| Source | `/falsify` release-readiness audit, 2026-08-21 (probe P5) | +| Trigger | A release is cut on or after **2026-10-18**, or anyone reports a red `test` check on `main` they cannot explain — read `tests/test_credential_expiry.py` before diagnosing anything else. | +| Owner | This repository for the documentation; the operator for the rotation that removes the cause. | +| Location | `tests/test_credential_expiry.py::test_the_platform_keys_are_not_about_to_expire`; the `protect_main` ruleset; `docs/operations/` (where the runbook that would say this does not exist). | + +Measured 2026-08-21 against the live ruleset: `protect_main` is **active**, the `test` job is a **required status check**, and `bypass_actors` is **empty** — C-86's finding, re-confirmed. `test_credential_expiry` fails from 30 days before the 2026-11-17 expiry, i.e. **2026-10-18**. From that date the required check is red, so no pull request merges to `main` and no release can be tagged. + +**This entry exists because it falls between two records and is in neither.** C-84 registers the expiry and the tripwire and does not mention the release path or required checks. C-86 registers that a red build cannot be bypassed and does not mention the tripwire. The interaction — *the guard we added will redden the check that cannot be bypassed, on a date we chose* — is the product of the two, and was not noticed when the tripwire was added four days earlier. The ruleset was never queried at the time; the audit queried it. + +**The escape is real and was verified, which is why this is Tier 3.** A pull request that sets `ACKNOWLEDGED_UNTIL` is green on its own branch (measured: `1 failed` → `6 passed`), so the block is not a trap and the fix is not gated behind the thing it fixes. What is missing is that **nothing tells a releaser any of this**. There is no release runbook in `docs/operations/`, and the failure message names `ACKNOWLEDGED_UNTIL` without saying that a release is what it is blocking. + +**Closing this is documentation, not code.** Either a release runbook that names the interaction, or a cross-reference in C-84 and C-86 so that whoever reads one meets the other. + +Cross-refs: **C-84** (the expiry and the tripwire), **C-86** (no way past a red build; zero bypass actors), **C-81** (what actually gates `main`). + +--- + +### C-112: Nothing here can see what production actually runs, and it ran a defect we had already fixed for eight days + +| Field | Value | +|-------|-------| +| ID | C-112 | +| Tier | 3 — nothing is silent: the defect that made this visible failed loudly and killed a delivery. What is missing is any signal about **version lag**, so work landing here does not reach production and no one on this side can tell. Coordination cost, not corruption. | +| Source | Cross-repo issue sweep, 2026-08-21 — prompted by the question *"have you checked all gh issues related to this repo?"*, which had not been done in this repository at all | +| Trigger | A release is cut here, **or** a delivery fails in a way this repository has already fixed — before diagnosing, read what `views-models`' launchers pin. If it is behind the newest tag, that is the first hypothesis, not the last. | +| Owner | This repository for the visibility; views-models for the pin itself. | +| Location | No file — that is the finding. `views-models` `postprocessors/{un_fao,un_crafd}/run.sh` hold `VIEWS_POSTPROCESSING_PIN`, and nothing in this repository reads them. | + +**Measured 2026-08-21.** Both launchers pin **1.1.0**. This repository's newest tag is **1.1.1**, published eight days earlier, and 1.1.0 carries the `_ContractStorePort.download` fail-open — register **C-99**, the defect that killed the first `un_crafd` delivery attempt on 2026-08-13. So production has been running a known-defective build of this package, on the FAO leg as well as CRAF'd, for over a week, and **nothing on this side could see it**. views-models#403 was filed for it and is open. + +At the same time, `main` carries **29 commits since 1.1.1** — every guard from this week's work, including the C-94 findability preflight and the C-105 torn-run ledger. None of it is in production either. + +**Two gaps, one shape: this repository cannot see the distance between what it declares and what anyone runs.** It does not know what consumers pin, and nothing compares its declared version against what is published. `tests/test_release_version.py` compares `pyproject` to the git tag — both facts *inside this repository* — which is why a version bump that is never tagged, or a tag no consumer ever takes, is invisible to it. + +**Why this is not C-111.** That entry is the outbound half — we change delivery behaviour and tell consumers nothing but an integer. This is the inbound half — we do not look at whether they took it. They compound: 1.1.1 fixed a delivery-killing defect, nothing announced it, and nothing checked whether it landed. + +**The obvious fix has a known cost, and it is already registered.** CI checks out sibling repositories so cross-repo assertions run (ADR-016); views-models is not among them. Adding it, plus a check that the launchers' pin is not behind the newest tag, would be the same shape as the existing drift checks. It would also add a fifth repository whose `main` can redden this build — **C-86**, no bypass actors. That trade is a decision, not a cleanup, which is why nothing is proposed here. + +**The wider observation, recorded once so it is not rediscovered.** Eighty-four open issues across the organisation mention `views-postprocessing`; this repository tracks none of them and had never been swept. Most are informational, several were filed *by* this seat, and a few carry live asks (views-faoapi#390, views-crafdapi#55, views-models#362). No mechanism is proposed for that either — but a sweep belongs in the next repo-assimilation rather than being found by accident at the end of a sprint. + +Cross-refs: **C-111** (the outbound half), **C-99** (the defect production is still running), **C-86** (the cost of adding another sibling to CI), **C-81** (what actually gates `main`), views-models#403. ## Disagreements @@ -3308,6 +3406,50 @@ Cross-refs: **views-faoapi C-161** (the deploy constraint §0.2a now surfaces --- +### C-111: A release can change whether a delivery fails, and the version number is the only thing that says so + +| Field | Value | +|-------|-------| +| ID | C-111 | +| Tier | 3 — nothing is silent and nothing corrupts; the cost lands on a consumer who takes a version that changes their pipeline's outcome with no notice, and on whoever then diagnoses it across two repositories. | +| Source | `/falsify` release-readiness audit, 2026-08-21 (discovered during execution, not predicted) | +| Trigger | The next version is cut — write what changed for a consumer, or record why the number alone is enough. | +| Owner | This repository. | +| Location | The repository root — there is no `CHANGELOG.md`, no `docs/operations/release_notes.md`, and no release notes on the existing tags. | + +The delta since tag `1.1.1` adds three exception types that can escape into a launcher: `delivery.findability.DeliveryNotFindableError` and `FindabilityUnverifiedError` (C-94), and `contract.source_metadata.ProducerClientUnavailable` (C-103). The first is the sharp one — **a delivery whose artifacts land somewhere the consumer cannot see previously succeeded silently and now raises.** + +That is the intended behaviour and the entire point of C-94. It is still a change a consumer must be told about, and the only signal views-models receives is a MINOR version bump in `VIEWS_POSTPROCESSING_PIN`. Nothing in this repository states that a previously-passing run can now fail. + +**The asymmetry is the finding.** This repository is unusually careful about telling *contributors* things — a register, ADRs, CICs, guards that refuse with paragraph-long explanations. It tells *consumers* nothing but an integer. views-models#403 is the shape of the consequence in the other direction: launchers sat on `1.1.0` for eight days after `1.1.1` fixed a defect that had already killed a delivery, because nothing made the difference legible. + +**Deliberately not proposed: a full changelog discipline.** What is needed is a line per release naming behaviour a consumer can observe. Whether that lives in `CHANGELOG.md`, in the GitHub release body, or in the pre-release notes FAO already receives is a choice, not a requirement. + +Cross-refs: **C-94** and **C-103** (the new failure modes), **C-112** (the inbound half — nothing checks whether a consumer took the release either; the two compound), **views-models#403** (the same gap costing eight days in the other direction), **C-24** (a consumer-facing contract divergence nobody surfaced). + +**✅ RESOLVED 2026-08-26, at the release its trigger named.** The trigger read *"the +next version is cut — write what changed for a consumer, or record why the number alone +is enough."* 1.2.0 is that release, and `CHANGELOG.md` now exists at the repository root +with an entry naming each of the three escaping exception types, the condition each one +replaces, and the new `observed_through` provenance field — under the heading the entry +asked for: **what changed for a consumer**, with the failure modes stated first. + +The venue was left open by the entry deliberately, and the choice made here is the root +`CHANGELOG.md` rather than the GitHub release body, so that a consumer reading the source +tree at a pinned version can see it without leaving the checkout. The release body quotes it. + +Two falsification probes asserted this gap and both are retired by this change — S2 from +the 2026-08-21 audit and H1 from the 2026-08-25 one. **They are the same finding, located +twice**, because the second audit designed its probes without reading the first's stubs. +That is a lesson about the audit procedure rather than about this entry, and it is recorded +in `tests/test_falsification_release_readiness.py` where the next auditor will meet it. + +**What this does not close: C-112**, the inbound half — nothing here can still see what a +consumer actually runs. A changelog tells views-models what changed; it does not tell this +repository whether views-models read it. + +--- + ## Resolved Disagreements ### D-07: Historical data route — keep pipeline-core dispatcher vs call datafactory directly — RESOLVED diff --git a/tests/fixtures/wire_contract/README.md b/tests/fixtures/wire_contract/README.md index 83bb000..bcab8c5 100644 --- a/tests/fixtures/wire_contract/README.md +++ b/tests/fixtures/wire_contract/README.md @@ -40,3 +40,12 @@ above it: the fixture was generated under `1.0.0`, CI has reproduced it continuo `1.6.0`, and `1.10.2` was verified to regenerate **all five artifacts byte-identically** before the pin was raised (2026-08-02). Raising it again does not require regenerating the fixture — but it does require proving that, the same way. + +**Confirmed across a `views_frames` MAJOR (2026-08-21).** The shard emitted through +`views_frames.io.arrow` under **1.10.2** and under **2.0.0** hashes identically, and both +equal the committed fixture (`203650fd…12c54`) — measured in an isolated environment at the +pinned toolchain (pyarrow 16.1.0, numpy 1.26.4), where all 61 frames-dependent tests also +pass at 2.0.0. So the views-frames 2.0.0 adoption (#286) is **not** a fixture re-vendor. It +is blocked only by views-pipeline-core, every published release of which (through 3.1.1) +pins `views-frames <2.0.0`. Recorded so the byte question is not re-opened when that +constraint widens. \ No newline at end of file diff --git a/tests/test_clone_readiness.py b/tests/test_clone_readiness.py index 54784d2..70be180 100644 --- a/tests/test_clone_readiness.py +++ b/tests/test_clone_readiness.py @@ -28,6 +28,7 @@ from __future__ import annotations +import ast import subprocess import sys import textwrap @@ -103,6 +104,40 @@ def _partner_prefixes() -> tuple[str, ...]: return tuple(f"views_postprocessing.{name}" for name in _PARTNER_PACKAGES) +def _imported_modules(path: Path, package: str) -> set[str]: + """Every absolute module name ``path`` imports, relative forms resolved. + + Three forms have to survive this, and the module docstring above names two of them + as the misses that made the earlier regex insufficient: + + import views_postprocessing.unfao.product + from views_postprocessing.unfao import product + from views_postprocessing import unfao <- the name is on the alias + from ..unfao import product <- the name is in `level` + + The last two are why this resolves `level` against the file's own package and joins + each alias onto the module. A first pass at this skipped both and would have passed + a manager importing its sibling relatively — the exact shape `contract/enrichment.py` + once used to demonstrate a real gap. + """ + parts = package.split(".") + found: set[str] = set() + for node in ast.walk(ast.parse(path.read_text())): + if isinstance(node, ast.Import): + found.update(alias.name for alias in node.names) + elif isinstance(node, ast.ImportFrom): + if node.level: + base = parts[: len(parts) - (node.level - 1)] + prefix = ".".join(base + ([node.module] if node.module else [])) + else: + prefix = node.module or "" + if not prefix: + continue + found.add(prefix) + found.update(f"{prefix}.{alias.name}" for alias in node.names) + return found + + def _modules_on_disk(package: str) -> set[str]: return { "views_postprocessing." + f.relative_to(_PKG).with_suffix("").as_posix().replace("/", ".") @@ -274,6 +309,70 @@ def test_the_machinery_does_not_pull_in_pipeline_core(): ) +@pytest.mark.parametrize("partner", _PARTNER_PACKAGES) +def test_a_partner_does_not_import_its_sibling(partner): + """The partners are independent, not merely both below the machinery. + + Everything else in this file proves the *vertical* arrows of ADR-002 — machinery + imports no partner, invariants import no machinery. Nothing proved the horizontal + one, and it is the arrow that keeps a partner liftable: `crafd/` and `unfao/` are + deliberate clones (register **C-33**), so the realistic violation is a copy-paste + that leaves a sibling's import behind. `test_the_machinery_imports_without_any_partner` + cannot see it — that test imports the machinery, and this would be partner-to-partner. + + Two halves, for the reason the module docstring already gives about regexes: the + subprocess is load-bearing and sees transitive arrivals; the source scan is the + supplement, and covers `managers/` — which the subprocess deliberately skips because + importing a manager needs views-pipeline-core, and a purity check should not be + contingent on a heavy framework being installed (C-40 (a)). + """ + siblings = tuple(f"views_postprocessing.{p}" for p in _PARTNER_PACKAGES if p != partner) + if not siblings: + pytest.skip("independence needs a sibling; only one partner is declared") + + # "managers" as a package SEGMENT, not a substring: a partner module named + # `managers_shared.py` would otherwise be dropped from this half while also sitting + # outside the AST half below, exempting it from the guard entirely with no signal. + # (`_modules_on_disk` already excludes `__init__.py`, so those arrive via the glob.) + importable = sorted( + m for m in _modules_on_disk(partner) if "managers" not in m.split(".") + ) + assert importable, f"no importable modules found for {partner}" + + result = _import_in_subprocess(tuple(importable), siblings) + assert result.returncode == 0, ( + f"{partner}'s own modules failed to import:\n{result.stderr}" + ) + leaked = [m for m in result.stdout.split("LEAKED:")[-1].strip().split(",") if m] + assert not leaked, ( + f"{partner} pulled in a sibling partner: {leaked}. The two are deliberate " + "clones (C-33) and must stay liftable one at a time — an import between them " + "means neither can be taken without the other, and no other test here sees it." + ) + + # IMPORTS only, via the AST — not a substring scan of the file. This repository's + # comments cite module paths constantly (C-33's own text points at + # `unfao/product.py`), so scanning the text would fail on documentation and get + # deleted for crying wolf, which is ADR-014 §3's whole point. + # + # EVERY file under `managers/`, not just `.py`: `managers/__init__.py` + # carries a real import today, and the subprocess half skips the whole package. + managers = sorted((_PKG / partner / "managers").rglob("*.py")) + assert managers, f"{partner} has no managers/ directory to scan" + for source in managers: + module = "views_postprocessing." + source.relative_to(_PKG).with_suffix("").as_posix().replace("/", ".") + package = module.rsplit(".", 1)[0] + offending = sorted( + name for name in _imported_modules(source, package) + if any(name == sib or name.startswith(sib + ".") for sib in siblings) + ) + assert not offending, ( + f"{source.relative_to(_REPO)} imports {offending}. These files are copies of " + "each other, so this is the shape a careless clone leaves behind — and it is " + "outside the subprocess half above, which skips managers/." + ) + + @pytest.mark.parametrize("partner", _PARTNER_PACKAGES) def test_the_guard_would_actually_catch_a_violation(partner): """A purity test that cannot fail is decoration. diff --git a/tests/test_falsification_release_readiness.py b/tests/test_falsification_release_readiness.py new file mode 100644 index 0000000..a076698 --- /dev/null +++ b/tests/test_falsification_release_readiness.py @@ -0,0 +1,120 @@ +"""Failing stubs from the release-readiness falsification audit, 2026-08-21. + +**DO NOT COMMIT THIS FILE AS-IS.** These tests fail by design, and this repository's +`protect_main` ruleset makes the `test` job a **required check with zero bypass +actors** — so committing red tests to `main` blocks every subsequent merge, including +the fix. That interaction is itself finding S1 below. + +Claim audited: *"we're ready to set up a PR, bump the version, and run the review +ritual"* — i.e. cutting 1.2.0 from current `main` would ship a correct, installable +package with every release guard satisfied and nothing in the repo's governance +blocking the path. + +Verdict: CONTESTED. No hard falsification; two soft ones, below. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parent.parent + + +@pytest.mark.xfail(reason="S1: unaddressed falsification — see the audit report", strict=True) +def test_the_release_path_survives_its_own_expiry_tripwire(): + """S1 (soft). From 2026-10-18 no release can be cut, and nothing says so. + + Measured 2026-08-21 against the live ruleset: `protect_main` is active, the `test` + job is a REQUIRED status check, and `bypass_actors` is **empty** — which is C-86's + finding, still true. `tests/test_credential_expiry.py` starts failing 30 days before + 2026-11-17, i.e. **2026-10-18**. From that date the required check is red, so no PR + merges to `main` and no release can be tagged. + + The escape is reachable — a PR that sets `ACKNOWLEDGED_UNTIL` is green on its own + branch (verified: 1 failed -> 6 passed) — which is why this is soft rather than + hard. What is missing is that **nothing tells a releaser this**. There is no release + runbook, and C-84 does not mention that its tripwire gates the release path. + + Fix: document the interaction where a releaser will meet it — a release runbook + under `docs/operations/`, or a line in C-84 and C-86 cross-referencing each other. + """ + runbook = list((_REPO / "docs" / "operations").glob("*release*")) + assert runbook, ( + "no release runbook exists, so the 2026-10-18 block on the release path is " + "recorded nowhere a releaser would look (C-84 x C-86)" + ) + text = "\n".join(p.read_text() for p in runbook) + assert "ACKNOWLEDGED_UNTIL" in text, ( + "the release runbook does not name the only in-repo way past the expiry " + "tripwire once it fires" + ) + + +# ───────────────────────────────────────────────────────────────────────────── +# Second audit, 2026-08-25. Claim: *"we are ready to bump the version, set up a +# PR to main, review, merge when review is good, then tag and publish."* +# +# Verdict: FALSIFIED — one hard, two soft. +# +# **H1 DISCHARGED 2026-08-26** (release 1.2.0). Its probe asserted that a release +# names what changed about failing for a consumer. `CHANGELOG.md` now exists and +# 1.2.0's entry names the three escaping exception types and the new provenance +# field, so the probe would XPASS and `strict=True` would turn that into a failure. +# Removed by hand rather than left to flip, per the S4 precedent (#200). Register +# C-111 is closed by the same change. +# +# **S2 DISCHARGED 2026-08-26** by the same change — and it is the same finding. +# S2 (2026-08-21) and H1 (2026-08-25) are one concern found twice by two audits, +# which is itself worth recording: the second audit did not read the first's stubs +# before designing probes. Both are C-111; both are closed by CHANGELOG.md. +# +# S1, S3 and S4 remain open and are below. The bump SIZE (minor, +# not patch) is recorded as an observation, not a falsification: nothing in the +# repo is wrong about it, it is a way the releaser could be. +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.xfail(reason="S3: unaddressed falsification — see the audit report", strict=True) +def test_tagging_actually_publishes(): + """S3 (soft). "Tag and publish" is not the mechanism this repo has. + + `.github/workflows/publish_package.yml` triggers on `release: published` and + `workflow_dispatch` — **not** on tag push. Pushing a tag runs nothing. + + The evidence that this is a live trap rather than a technicality: tags `1.0.0` + and `1.1.0` both exist and **neither has a GitHub Release**. Only `1.1.1` does, + which is the only version this workflow has ever published. + + Fails until the workflow triggers on tag push, or until a release runbook states + that cutting a GitHub Release — not tagging — is the publishing step. + """ + wf = (_REPO / ".github/workflows/publish_package.yml").read_text() + assert "tags:" in wf or "push:" in wf, ( + "publish triggers only on `release: published`; a plan that says 'tag, then " + "publish' will tag and stop, and nothing will say so" + ) + + +@pytest.mark.xfail(reason="S4: unaddressed falsification — see the audit report", strict=True) +def test_the_publish_job_cannot_ship_untested_code(): + """S4 (soft). The publish job runs no tests and declares no dependency. + + `publish_package.yml` has no `needs:`, no pytest step, and one gate: that the + version in `pyproject.toml` parses higher than the newest on PyPI. So a GitHub + Release cut from any commit — a branch, a stale `main`, a commit whose `test` + job failed — builds and uploads to PyPI unconditionally. + + Nothing has gone wrong yet because releases have been cut from a green `main` + by hand. The guard is the habit, not the workflow, and habits are what C-86 + already showed this repo cannot rely on when one person holds them. + + Fails until the publish job depends on a passing test run, or refuses a ref + whose checks are not green. + """ + wf = (_REPO / ".github/workflows/publish_package.yml").read_text() + assert "needs:" in wf or "pytest" in wf, ( + "publish validates only version-greater-than-PyPI; nothing establishes that " + "the code being shipped passes its own suite" + ) diff --git a/tests/test_input_integrity_e2e.py b/tests/test_input_integrity_e2e.py index 8e0a5a8..20a9361 100644 --- a/tests/test_input_integrity_e2e.py +++ b/tests/test_input_integrity_e2e.py @@ -100,6 +100,7 @@ def test_s5_upload_description_carries_structured_provenance(): expected_cell_count=coverage.expected_for("land_gaul"), actual_cell_count=3, unmapped_count=0, + observed_through=559, ) description = f"Enriched ... provenance={json.dumps(prov, separators=(',', ':'))}" @@ -120,5 +121,6 @@ def test_s5_provenance_records_unmapped_cells_when_present(): expected_cell_count=coverage.expected_for("land_gaul"), actual_cell_count=3, unmapped_count=1, + observed_through=559, ) assert prov["unmapped_count"] == 1 diff --git a/tests/test_observed_range_provenance.py b/tests/test_observed_range_provenance.py new file mode 100644 index 0000000..7fdc14f --- /dev/null +++ b/tests/test_observed_range_provenance.py @@ -0,0 +1,188 @@ +"""#297: the artifact must say what boundary it clipped against. + +CRAF'd received July 2026 as history. The month was real but ~1% reported — six +cells of 64,742 — and the producer's *inferred* boundary (a month counts as +observed once its slice sums above zero) therefore declared it observed. The clip +kept it, correctly, by its own contract. + +Establishing that took a day, because the delivered artifact could not answer the +one question a partner asks afterwards: *observed through when, and decided against +what?* The boundary was only recoverable at all because July's six cells happened to +land in ``ged_ns``/``ged_os`` rather than ``ged_sb``, leaving a non-zero trace. Had +they landed in ``ged_sb``, the data would have been mute. + +So the boundary is stamped, and stamped **unconditionally**. The degrade-open case — +boundary unreadable, clip skipped, unobserved months may be present — is the case that +most needs recording, and it is exactly the case an "omit when absent" field would drop. + +Two layers, as the repo tests every other delivery invariant: the rule on primitives, +and the wiring as declaration checks (constructing a manager needs pipeline-core, a +views-models path manager and a live Appwrite environment — C-40). +""" + +from __future__ import annotations + +import ast +import json +from pathlib import Path + +import pytest + +from tests.conftest import PARTNER_PACKAGES +from views_postprocessing.delivery.provenance import ( + DESCRIPTION_MAX, + UNREAD, + build_provenance, + compact_description, +) + +_REPO = Path(__file__).resolve().parent.parent + + +def _prov(**over): + base = dict( + lookup_version="gaul-2024a", + region="land_gaul", + expected_cell_count=64742, + actual_cell_count=64742, + unmapped_count=0, + observed_through=559, + ) + base.update(over) + return build_provenance(**base) + + +# ── the rule ─────────────────────────────────────────────────────────────── + + +def test_the_boundary_the_clip_used_is_stamped(): + assert _prov(observed_through=559)["observed_through"] == 559 + + +def test_a_skipped_clip_is_stamped_as_null_rather_than_omitted(): + """The #297 property. An absent key is indistinguishable from an artifact + built before this field existed; an explicit null says "the boundary could + not be read, so this delivery was NOT clipped".""" + prov = _prov(observed_through=None) + assert "observed_through" in prov + assert prov["observed_through"] is None + assert json.loads(compact_description(prov))["observed_through"] is None + + +def test_building_provenance_without_reading_the_boundary_refuses(): + """UNREAD is a call-order bug, not a delivery condition. It must not + silently become null — that would report "clip skipped" for a run whose + clip in fact ran.""" + with pytest.raises(ValueError, match="never read"): + _prov(observed_through=UNREAD) + + +def test_unread_is_distinct_from_none(): + assert UNREAD is not None + assert not isinstance(None, type(UNREAD)) + + +def test_the_boundary_survives_the_compaction_fallback(): + """``compact_description`` drops non-essential keys when the 255-char carrier + overflows. A boundary that vanishes precisely when the description is long is + a guard that disappears when it is needed, so it belongs in the essential set.""" + # The padding must overflow the full dict while leaving the essential set inside + # the limit. Measured 2026-08-25: the fallback triggers from 104 chars and the + # essential set still fits to ~117. Both assertions below fail loudly if that + # window ever moves, so the constant cannot drift silently into a vacuous test. + prov = _prov(lookup_version="g" * 110, fill_count=3) + text = compact_description(prov) + assert len(text) <= DESCRIPTION_MAX + round_trip = json.loads(text) + assert "fill_count" not in round_trip, "the fallback did not trigger; test is vacuous" + assert round_trip["observed_through"] == 559 + + +# ── the wiring ───────────────────────────────────────────────────────────── + + +def _manager_source(partner: str) -> str: + return (_REPO / "views_postprocessing" / partner / "managers" / f"{partner}.py").read_text() + + +def _func(source: str, name: str) -> ast.FunctionDef: + return next( + n for n in ast.walk(ast.parse(source)) + if isinstance(n, ast.FunctionDef) and n.name == name + ) + + +@pytest.mark.parametrize("partner", PARTNER_PACKAGES) +def test_the_manager_initialises_the_boundary_as_unread(partner): + init = _func(_manager_source(partner), "__init__") + assigns = [ + n for n in ast.walk(init) + if isinstance(n, ast.Assign) + and any( + isinstance(t, ast.Attribute) and t.attr == "_observed_through" + for t in n.targets + ) + ] + assert assigns, f"{partner}: __init__ does not initialise _observed_through" + src = ast.unparse(assigns[0].value) + assert "UNREAD" in src, ( + f"{partner}: _observed_through initialises to {src!r}, not UNREAD. " + "Initialising to None makes 'never read' indistinguishable from 'read and " + "unavailable' — the exact conflation #297 exists to prevent." + ) + + +@pytest.mark.parametrize("partner", PARTNER_PACKAGES) +def test_the_boundary_is_recorded_before_the_degrade_open_return(partner): + """The assignment must precede the ``if lv is None: return`` early exit. + + Placed after it, the degrade-open path — the one that most needs recording — + would leave the attribute UNREAD and the delivery would raise at provenance + time instead of reporting that it did not clip. + """ + read = _func(_manager_source(partner), "_read_historical_frame") + assigns = [ + n for n in ast.walk(read) + if isinstance(n, ast.Assign) + and any( + isinstance(t, ast.Attribute) and t.attr == "_observed_through" + for t in n.targets + ) + ] + assert assigns, f"{partner}: _read_historical_frame never records the boundary" + + returns = [n for n in ast.walk(read) if isinstance(n, ast.Return)] + assert returns, f"{partner}: expected an early return in _read_historical_frame" + first_return = min(n.lineno for n in returns) + assert min(a.lineno for a in assigns) < first_return, ( + f"{partner}: _observed_through is assigned at or after the early return, so " + "the degrade-open path would never record that the clip was skipped." + ) + + +@pytest.mark.parametrize("partner", PARTNER_PACKAGES) +def test_the_manager_passes_the_boundary_into_provenance(partner): + desc = _func(_manager_source(partner), "_historical_frame_description") + kwargs = { + kw.arg + for c in ast.walk(desc) + if isinstance(c, ast.Call) + for kw in c.keywords + if kw.arg + } + assert "observed_through" in kwargs, ( + f"{partner}: _historical_frame_description builds provenance without the " + "boundary. build_provenance requires it, so this would raise at delivery." + ) + + +def test_both_partners_carry_it_identically(): + """#297 was filed as a CRAF'd defect; the two managers are deliberate clones + and the UN-FAO side has an external partner. A fix in one only is half a fix.""" + counts = { + p: _manager_source(p).count("_observed_through") for p in PARTNER_PACKAGES + } + assert len(set(counts.values())) == 1, ( + f"the partners diverge on the boundary stamp: {counts}" + ) + assert all(v >= 3 for v in counts.values()), counts diff --git a/tests/test_provenance.py b/tests/test_provenance.py index 0ac3355..08e2b91 100644 --- a/tests/test_provenance.py +++ b/tests/test_provenance.py @@ -15,9 +15,13 @@ def test_carries_all_core_fields_from_primitives(): expected_cell_count=64_742, actual_cell_count=64_742, unmapped_count=0, + observed_through=559, ) assert prov == { "lookup_version": "v1.4.0", + # #297: always present, never omitted — an absent boundary is + # indistinguishable from an artifact built before the field existed. + "observed_through": 559, "region": "land_gaul", "expected_cell_count": 64_742, "actual_cell_count": 64_742, @@ -32,6 +36,7 @@ def test_fill_count_omitted_when_not_supplied(): expected_cell_count=1, actual_cell_count=1, unmapped_count=0, + observed_through=559, ) assert "fill_count" not in prov @@ -43,6 +48,7 @@ def test_fill_count_included_when_supplied(): expected_cell_count=1, actual_cell_count=1, unmapped_count=0, + observed_through=559, fill_count=7, ) assert prov["fill_count"] == 7 @@ -55,6 +61,7 @@ def test_unpinned_region_keeps_none_expected_count(): expected_cell_count=None, actual_cell_count=13_110, unmapped_count=0, + observed_through=559, ) assert prov["expected_cell_count"] is None assert prov["region"] == "africa_me_legacy" @@ -67,6 +74,7 @@ def test_result_is_json_serializable(): expected_cell_count=64_742, actual_cell_count=64_700, unmapped_count=0, + observed_through=559, fill_count=3, ) # round-trips cleanly — it must survive serialization into the upload description. diff --git a/tests/test_redaction_guard.py b/tests/test_redaction_guard.py index 62e2b26..7f163ad 100644 --- a/tests/test_redaction_guard.py +++ b/tests/test_redaction_guard.py @@ -141,10 +141,17 @@ def test_provenance_carries_only_the_declared_closed_keyset(): expected_cell_count=64742, actual_cell_count=64742, unmapped_count=0, + observed_through=559, fill_count=3, ) assert set(prov) == { "lookup_version", + # #297: the producer's observed-data frontier this run clipped against. A + # month_id integer — no PII, no credential, no internal path — and it is + # precisely what the partner needs to tell a sparsely-reported month from + # a fabricated one. Widening this keyset is a deliberate act; that is why + # this guard exists. + "observed_through", "region", "expected_cell_count", "actual_cell_count", diff --git a/tests/test_selection_guard.py b/tests/test_selection_guard.py index 1cb87bb..b113ba0 100644 --- a/tests/test_selection_guard.py +++ b/tests/test_selection_guard.py @@ -98,6 +98,7 @@ def test_compact_description_fits_the_store_limit(): expected_cell_count=64742, actual_cell_count=64742, unmapped_count=0, + observed_through=559, ) text = compact_description(prov) assert len(text) <= DESCRIPTION_MAX diff --git a/views_postprocessing/crafd/managers/crafd.py b/views_postprocessing/crafd/managers/crafd.py index 5201056..d5f9982 100644 --- a/views_postprocessing/crafd/managers/crafd.py +++ b/views_postprocessing/crafd/managers/crafd.py @@ -156,6 +156,9 @@ def __init__( logger.info(f"Initializing {self.__class__.__name__}") self._forecast_resolution = None # {target: TargetLease}, set by _read self._historical_frame = None # views_frames.FeatureFrame, set by _read + # int | None, set by _read_historical_frame. UNREAD until then, so that + # "never read" cannot be mistaken for "read and unavailable" (#297). + self._observed_through = provenance.UNREAD def _read_historical_frame(self): """#126: historical actuals as a views_frames.FeatureFrame — the first @@ -179,6 +182,10 @@ def _read_historical_frame(self): exc_info=True, ) lv = None + # Stamped into provenance either way: the boundary this run clipped against, + # or None meaning the clip was skipped. #297 cost a day of forensics because + # the artifact could not answer which. + self._observed_through = lv if lv is None: self._historical_frame = frame return @@ -417,6 +424,7 @@ def _historical_frame_description(self, table, timestamp: str) -> str: expected_cell_count=coverage.expected_for(region), actual_cell_count=len(frame_extraction.cells_of(self._historical_frame)), unmapped_count=historical.unmapped_cell_count(table), + observed_through=self._observed_through, ) return provenance.compact_description(prov) diff --git a/views_postprocessing/delivery/provenance.py b/views_postprocessing/delivery/provenance.py index 35adab8..a7cc049 100644 --- a/views_postprocessing/delivery/provenance.py +++ b/views_postprocessing/delivery/provenance.py @@ -10,11 +10,40 @@ a free-text ``description`` — so the manager serializes this dict into ``description`` as JSON for now. A dedicated field is requested upstream (see C-15); when it lands, only the manager's attach step changes, not this shape. + +Deferred deliberately (#297): the producer also publishes a **per-source** boundary map, +``last_valid_month_ids``, in the store attrs and consumer manifest. It is not stamped here +because ``datafactory_query.defaults`` exposes only the scalar ``get_last_valid_month_id``, +and a multi-source map would not fit the 255-char carrier below in any case. +**Trigger:** when C-15's structured metadata field lands upstream and the 255-char ceiling +goes with it, stamp the per-source map alongside the scalar. """ from __future__ import annotations +class _Unread: + """Sentinel for "the observed-range boundary has not been read yet". + + Distinct from ``None``, which means "read attempted and unavailable, so this + delivery was NOT clipped". Conflating the two is the C-103 mistake — a broken + read reported as a producer that publishes no boundary — and #297 is what it + costs: an artifact that cannot say what it clipped against, and a day of + forensics to recover a number the delivery already had. + """ + + __slots__ = () + + def __repr__(self) -> str: # pragma: no cover - diagnostic only + return "" + + +#: Managers initialise their boundary attribute to this; ``build_provenance`` +#: refuses it. Reaching provenance without having read the boundary is a bug in +#: the call order, not a delivery condition. +UNREAD = _Unread() + + def build_provenance( *, lookup_version: str, @@ -22,6 +51,7 @@ def build_provenance( expected_cell_count: int | None, actual_cell_count: int, unmapped_count: int, + observed_through: int | None | _Unread, fill_count: int | None = None, ) -> dict: """Assemble the structured provenance for one delivered file. @@ -37,13 +67,27 @@ def build_provenance( expected_cell_count: the region's pinned cell count, or None if unpinned. actual_cell_count: distinct cells actually delivered in this file. unmapped_count: delivered cells with missing metadata (0 once validation passes). + observed_through: the producer's ``last_valid_month_id`` this delivery clipped + against, or ``None`` if the boundary could not be read and the clip was + therefore **skipped** (degrade-open, C-26). Always emitted, never omitted: + an absent field is indistinguishable from an older artifact that never + stamped one, and that ambiguity is the whole of #297. Passing + :data:`UNREAD` raises. fill_count: optional count of fabricated/filled values, if known. Returns: A JSON-serializable dict of the provenance fields. """ + if isinstance(observed_through, _Unread): + raise ValueError( + "observed_through was never read — provenance is being built before the " + "observed-range boundary was fetched. This is a call-order bug, not a " + "delivery condition: pass the boundary the clip used, or None if the read " + "failed and the clip was skipped (C-26)." + ) provenance: dict = { "lookup_version": lookup_version, + "observed_through": observed_through, "region": region, "expected_cell_count": expected_cell_count, "actual_cell_count": actual_cell_count, @@ -70,7 +114,14 @@ def compact_description(prov: dict) -> str: if len(text) > DESCRIPTION_MAX: essential = { k: prov[k] - for k in ("lookup_version", "region", "expected_cell_count", "actual_cell_count", "unmapped_count") + for k in ( + "lookup_version", + "observed_through", + "region", + "expected_cell_count", + "actual_cell_count", + "unmapped_count", + ) if k in prov } text = json.dumps(essential, separators=(",", ":")) diff --git a/views_postprocessing/unfao/managers/unfao.py b/views_postprocessing/unfao/managers/unfao.py index 4d6ddf6..8720230 100644 --- a/views_postprocessing/unfao/managers/unfao.py +++ b/views_postprocessing/unfao/managers/unfao.py @@ -156,6 +156,9 @@ def __init__( logger.info(f"Initializing {self.__class__.__name__}") self._forecast_resolution = None # {target: TargetLease}, set by _read self._historical_frame = None # views_frames.FeatureFrame, set by _read + # int | None, set by _read_historical_frame. UNREAD until then, so that + # "never read" cannot be mistaken for "read and unavailable" (#297). + self._observed_through = provenance.UNREAD def _read_historical_frame(self): """#126: historical actuals as a views_frames.FeatureFrame — the first @@ -179,6 +182,10 @@ def _read_historical_frame(self): exc_info=True, ) lv = None + # Stamped into provenance either way: the boundary this run clipped against, + # or None meaning the clip was skipped. #297 cost a day of forensics because + # the artifact could not answer which. + self._observed_through = lv if lv is None: self._historical_frame = frame return @@ -417,6 +424,7 @@ def _historical_frame_description(self, table, timestamp: str) -> str: expected_cell_count=coverage.expected_for(region), actual_cell_count=len(frame_extraction.cells_of(self._historical_frame)), unmapped_count=historical.unmapped_cell_count(table), + observed_through=self._observed_through, ) return provenance.compact_description(prov)