diff --git a/README.md b/README.md index ff12b06..b9b74f8 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,12 @@ The pandas metadata-join and history-clip stages were retired with the legacy de in #149; their rules survive as called invariants under `delivery/`. See the [manager README](views_postprocessing/unfao/managers/README.md) for what moved where. +### If a delivered value turns out to be wrong + +`docs/operations/correction_procedure.md` — how to establish which deliveries are +affected, confirm the fault offline, and supersede on the wire. The contract has no +retraction primitive; a correction is a new complete run, manifest last. + ### Output schema (geographic metadata columns) These 9 columns are the delivered geography contract (declared in `contract/gaul_schema.py`): diff --git a/docs/CLONING.md b/docs/CLONING.md index e149ce0..40145e8 100644 --- a/docs/CLONING.md +++ b/docs/CLONING.md @@ -80,6 +80,17 @@ used to select a retired code path silently (register C-63). 4. Run `pytest tests/test_clone_readiness.py` in the clone. If it fails, the boundary moved. +## Before your first delivery, not after + +**Answer your partner's correction questions before you ship to them, not after a bad +delivery.** `docs/operations/correction_procedure.md` is FAO's, and its steps 1–3 and 5 +transfer unchanged — they are contract mechanics, not partner specifics. **Step 4 does +not:** who contacts your partner, through what channel, how fast, and whether they +expect a retraction or a supersession are answers only your partner can give. + +This repo shipped run-0 to the UN FAO on 2026-07-27 with that step undecided (register +C-22), and it is still undecided. Do not inherit that. + ## Where the reasoning lives `docs/ADRs/013_sampled_forecast_wire_contract.md` — the wire contract · diff --git a/docs/operations/correction_procedure.md b/docs/operations/correction_procedure.md new file mode 100644 index 0000000..2161402 --- /dev/null +++ b/docs/operations/correction_procedure.md @@ -0,0 +1,164 @@ +# Correcting a delivery that has already reached the UN FAO + +**Read this when a delivered value is suspect.** It is a runbook, not an argument — a +person following it at 22:00 after a bad delivery should not have to read three other +documents first. Where a step needs background, the link is inline and optional. + +Register **C-22**. Supersedes issue #15, whose procedure described the pre-2026-07 +delivery (disk caches, shapefiles) and no longer applies to anything. + +> **Status: steps 1–3 and 5 are executable today. Step 4 is not** — nobody has decided +> who contacts the UN FAO, or whether FAO expects a retraction or a supersession. Those +> are the operator's calls and are stated in §4 exactly as they need to be answered. + +--- + +## 0. What a delivery is, in one paragraph + +A delivery is one **run**, identified by a `run_id` such as +`rusty_bucket_forecasting_20260727_095355`. It consists of arrow **shards** (one per +target per month), one GAUL **sidecar**, one **historical artifact**, and one **run +manifest** — uploaded **last**, deliberately, because it is the *commit marker* +(ADR-013 §4). A run whose upload died halfway has no manifest and is **invisible to +consumers**, which is the contract's atomicity mechanism and the first thing to +understand before touching anything. + +--- + +## 1. Which deliveries are affected? + +Two fields identify a delivery, and both are on it by construction: + +| field | what it answers | where it lives | +|---|---|---| +| `run_id` | *which run* | the manifest, every shard's header, and each store document's filename | +| `lookup_version` | *which GAUL build produced its geography* | the historical artifact's store-document `description` (a compact JSON provenance record) | + +`lookup_version` has the form `land_gaul@f74d3b2b` — region, then the digest of the +views-datafactory ingestion the lookup was built from. **It cannot silently be +`"unknown"`**: since register C-60 the reader raises rather than degrading, so a stamp +you can read is a stamp you can trust. + +**To enumerate affected runs:** + +1. If the fault is in **geography** (a wrong country, admin unit or coordinate), the + blast radius is *every delivery sharing the suspect `lookup_version`*. Compare the + stamp on each delivered run's provenance record. +2. If the fault is in **forecast values**, the blast radius is the runs whose manifest + names the affected producer run — the manifest carries `run_id` and `targets`. +3. If the fault is in **coverage** (missing or extra cells), the manifest's + `expected_cell_count` and the provenance record's `actual_cell_count` bound it + without opening a single parquet. + +Do this before touching the store. A correction whose scope you have not established +is a second incident. + +--- + +## 2. Reproduce and confirm, offline + +**Do not re-run the pipeline to investigate.** The committed artifacts are enough, and +re-running changes the thing you are diagnosing. + +``` +pytest -q tests/test_gaul_lookup_fidelity.py +``` + +26 tests. The always-on half checks the committed lookup for key uniqueness, region-set +equality, coordinate correctness, and the absence of nulls and `-1` sentinels. With a +views-datafactory checkout present (`VIEWS_DATAFACTORY`), the second half compares all +seven GAUL columns against the producer's own parquets. + +**If those pass and a value is still wrong, the fault is upstream, not here.** That +distinction is register C-43's and it is load-bearing: this repository is verified to +carry views-datafactory's answer faithfully; whether that answer is *right* — in +particular the degree-based area-majority join at high latitudes — is +**views-datafactory#387**. Do not correct a delivery to compensate for a producer +defect; fix the producer and rebuild. + +--- + +## 3. Correct on the wire + +**The contract has no retraction primitive. It has supersession**, and that is +deliberate: the manifest-last commit ordering means the way to replace a run is to +publish a *new complete run*, not to mutate an old one. + +1. Fix the cause — the lookup, the producer, or the code — and land it. +2. Rebuild any affected artifact. For the lookup: + `PYTHONPATH=. python scripts/build_gaul_lookup.py --datafactory --region land_gaul` + The builder now **refuses** to produce an artifact it cannot stamp, so a rebuild is + either traceable or it fails. +3. Re-run the delivery. It publishes a new `run_id`, shards → sidecar → **manifest last**. +4. **Do not delete the superseded run's files before the consumer has moved.** The old + manifest is what makes the old run selectable; removing shards while it stands + produces a run that resolves and then fails to load, which is worse than a wrong + value because it looks like an outage. + +### What the consumer will actually pick up — read this before assuming + +views-faoapi resolves **the newest manifest matching a broad filter** +(`wire/source_selection.py`). So a corrected run is picked up because it is *newer*, not +because it is *correct*. Two consequences: + +- **Publishing the correction is what switches the consumer over.** There is no + "activate" step and no rollback — republishing the old run would mean publishing it + again under a new id. +- **Recency-based selection is register C-73**, open, and fixed upstream in + views-pipeline-core 3.0.0 but not yet taken here. **#133** would have the manifest + declare `{maturity, source, required-schema-version}` so a consumer could select on + intent rather than on timestamp. Until then, do not publish a *test* or *partial* + correction to the production bucket — the consumer cannot tell it from the real one. + +--- + +## 4. Telling the UN FAO — **not decided; the operator must answer this** + +Everything above can be executed by whoever is on the keyboard. This cannot, and it is +the step that matters most to the partner. + +**Two questions, in plain language:** + +> **1. When a delivery is found to be wrong, who contacts the UN FAO, through what +> channel, and how quickly?** +> Right now nobody has said. There is no named person, no address, and no expectation +> about timing — so in practice the answer would be improvised by whoever noticed, +> under time pressure, which is the worst moment to invent a process. + +> **2. Does the UN FAO expect us to *retract* the bad delivery, or to *supersede* it?** +> These need different behaviour. Supersession is what the contract does today: the old +> run stays in the bucket and a newer one wins. Retraction would mean removing or +> marking the old run so it cannot be served — which the wire has no mechanism for, and +> which would need an ADR-013 amendment and agreement from views-faoapi. +> **This is a question for them, not a decision for us.** + +Per `CLAUDE.md`, anything touching an external party is the operator's call. Until both +are answered, treat this step as: **stop, and ask Simon.** Do not contact the partner +ad hoc; an inconsistent first message is harder to correct than a slow one. + +--- + +## 5. Preserve the evidence + +A retraction that destroys the evidence makes the post-mortem impossible. Keep, at +minimum: + +- the superseded run's `run_id`, its manifest, and its provenance record +- the `lookup_version` in force at the time, and the lookup artifact that produced it + (it is committed to this repo, so the git history already holds it) +- what was wrong, how it was found, and which check failed to catch it + +The last one is the point. Every silent-failure entry in +`reports/technical_risk_register.md` exists because something shipped that no gate +objected to. A correction that does not record *which gate was missing* buys a fix and +leaves the hole. + +--- + +## See also + +`docs/ADRs/013_sampled_forecast_wire_contract.md` §4 (commit ordering), §11 (transition +rules) · `views_postprocessing/delivery/provenance.py` (the fields a delivery carries) · +`views_postprocessing/contract/wire/source_selection.py` (how the consumer chooses) · +register **C-22**, **C-43**, **C-60**, **C-73** · issues **#15**, **#131**, **#133**, +views-datafactory **#387** diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index a0c0a9e..c11f869 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 | 16 | -| Resolved Concerns | 58 | +| Open Concerns | 15 | +| Resolved Concerns | 59 | --- @@ -173,24 +173,6 @@ See also C-14 (stale cache without version tracking), C-22 (no post-delivery cor --- -### C-22: No post-delivery correction process for wrong assignments - -| Field | Value | -|-------|-------| -| ID | C-22 | -| Tier | 3 | -| Source | `falsification-audit` (2026-06-02) | -| Trigger | When the run-0 integrity verification (#131 q1) or any FAO/faoapi query surfaces a suspect delivered value — follow the correction procedure; **issue #15 must produce one first.** Re-check at every subsequent delivery until it exists. | -| Location | `views_postprocessing/unfao/managers/unfao.py:442-494` (`_save_contract`), `:518-578` (legacy `_save`); issue #15 (the undocumented procedure) | - -The delivery chain has four stages beyond the code: Appwrite bucket → UN FAO download → FAO systems → operational decisions. When an error is discovered post-delivery, correction requires clearing cache, re-running, re-uploading, notifying FAO, and FAO retracting old data. Steps 3-5 have no documented procedure. - -Part of Cluster B (operational impact dimension). See also C-14 (RESOLVED — mapper-era cache), C-15. - -**Update 2026-07-31 (review-rr — the conditional is spent):** this entry was written conditionally — "*if* wrong data ever reaches FAO." **Run-0 delivered on 2026-07-27** (108 arrow shards + sidecar + manifest to `unfao_bucket`, plus 28,356,996 historical rows at 64,742 cells), and its integrity verification is still open (#131 q1). There is now delivered, unverified data in the partner's store and still no documented correction/recall procedure. Tier held at 3 (process gap, no code defect), but this is the acute member of Cluster J — **issue #15 is now the blocking artifact, not a nice-to-have.** - ---- - ### C-24: Postprocessor output schema diverges from FAO-confirmed API contract | Field | Value | @@ -584,6 +566,41 @@ See also C-40 (the inheritance/representation coupling this migration unwinds), ## Resolved Concerns +### C-22: No post-delivery correction process for wrong assignments — RESOLVED (procedure written; the partner-facing step is an open OPERATOR decision) + +| Field | Value | +|-------|-------| +| ID | C-22 | +| Resolved | 2026-08-02 | +| Resolution | **Closed by S8 (#189)** — `docs/operations/correction_procedure.md`, written against the delivery that exists rather than the one #15 described in June (disk caches and shapefiles, both deleted with the runtime mapper). + +**What it establishes.** Affected deliveries are identified by `run_id` and `lookup_version`, both present by construction — and `lookup_version` can no longer be the string `"unknown"`, because **C-60** made the reader raise instead of degrading. That is the dependency this story waited on: a procedure whose identification step rests on a field that can silently become a placeholder is not a procedure. Confirmation is offline against committed artifacts (`tests/test_gaul_lookup_fidelity.py`, 26 tests), so investigating does not change the thing being investigated. + +**The wire mechanism is supersession, not retraction**, and the document says so plainly rather than inventing one: manifest-last commit ordering (ADR-013 §4) means a run is replaced by publishing a new complete run. It also states the consequence a reader would otherwise discover the hard way — views-faoapi selects **the newest manifest over a broad filter**, so a correction is picked up because it is *newer*, not because it is *correct*, and a test or partial correction published to the production bucket is indistinguishable from the real one. That is **C-73**, cited rather than re-solved, with **#133** named as the fix that would let a consumer select on intent. + +Pinned by three checks in `tests/test_doc_accuracy.py`: the document exists and names the identification fields; it describes ADR-013 mechanisms and none of the deleted ones; and it still flags its undecided step. + +**⚠ RESIDUAL — one step is written but NOT decided, and it is the step that reaches the partner.** Two questions belong to the operator (`CLAUDE.md`: anything touching an external party): + +1. **Who contacts the UN FAO when a delivery is found wrong, through what channel, and how fast?** No named person, no address, no timing expectation. In practice it would be improvised by whoever noticed, under time pressure. +2. **Does FAO expect retraction or supersession?** Supersession is what the contract does. Retraction has **no wire mechanism** and would need an ADR-013 amendment plus agreement from views-faoapi. It is a question for them, not a decision for us. + +The document states both verbatim and instructs the reader to stop and ask rather than improvise. **C-22 closes because the procedure now exists and says exactly where it stops**; what remains is a decision, not engineering. Registered as the standing gap rather than left as an open concern that would read as unfinished work. + +`docs/CLONING.md` carries the same warning forward: a clone should answer its partner's correction questions **before** first delivery. This repo shipped run-0 on 2026-07-27 with that step undecided, and it still is. | +| Tier | 3 | +| Source | `falsification-audit` (2026-06-02) | +| Trigger | When the run-0 integrity verification (#131 q1) or any FAO/faoapi query surfaces a suspect delivered value — follow the correction procedure; **issue #15 must produce one first.** Re-check at every subsequent delivery until it exists. | +| Location | `views_postprocessing/unfao/managers/unfao.py:442-494` (`_save_contract`), `:518-578` (legacy `_save`); issue #15 (the undocumented procedure) | + +The delivery chain has four stages beyond the code: Appwrite bucket → UN FAO download → FAO systems → operational decisions. When an error is discovered post-delivery, correction requires clearing cache, re-running, re-uploading, notifying FAO, and FAO retracting old data. Steps 3-5 have no documented procedure. + +Part of Cluster B (operational impact dimension). See also C-14 (RESOLVED — mapper-era cache), C-15. + +**Update 2026-07-31 (review-rr — the conditional is spent):** this entry was written conditionally — "*if* wrong data ever reaches FAO." **Run-0 delivered on 2026-07-27** (108 arrow shards + sidecar + manifest to `unfao_bucket`, plus 28,356,996 historical rows at 64,742 cells), and its integrity verification is still open (#131 q1). There is now delivered, unverified data in the partner's store and still no documented correction/recall procedure. Tier held at 3 (process gap, no code defect), but this is the acute member of Cluster J — **issue #15 is now the blocking artifact, not a nice-to-have.** + +--- + ### C-71: `appwrite_env.assert_env_declared` raises without logging — ADR-008 non-compliance in an entry-validation seam — RESOLVED | Field | Value | diff --git a/tests/test_doc_accuracy.py b/tests/test_doc_accuracy.py index de7500b..6a81cee 100644 --- a/tests/test_doc_accuracy.py +++ b/tests/test_doc_accuracy.py @@ -296,3 +296,63 @@ def test_the_legacy_ok_marker_still_works(tmp_path): "a marker on the NEXT line excused the mention — the opt-out must stay line-scoped, " "or a single marker silently covers a whole document" ) + + +# --- 4. the post-delivery correction procedure (S8 / #189, register C-22) --------------- + +_CORRECTION = _REPO / "docs" / "operations" / "correction_procedure.md" + + +def test_the_correction_procedure_exists_and_names_how_to_identify_a_delivery(): + """C-22's trigger fired on 2026-07-27 and there was no written procedure. + + Run-0 put 64,742 cells x 36 months into a partner's store, live. The procedure + that existed (#15, June) described disk caches and shapefiles — both deleted with + the runtime mapper. This pins the two things a correction cannot start without: + the fields that say *which* delivery is affected. + + Mirrors ``test_clone_readiness.py::test_the_cloning_guide_exists_and_names_what_ + must_be_supplied`` — the human half of a guarantee, checked mechanically. + """ + assert _CORRECTION.exists(), f"the correction procedure is missing: {_CORRECTION}" + text = _CORRECTION.read_text() + for required in ("run_id", "lookup_version", "manifest"): + assert required in text, ( + f"the correction procedure does not mention {required!r} — without it a " + "reader cannot establish which deliveries are affected before acting" + ) + + +def test_the_correction_procedure_describes_the_delivery_that_exists(): + """It must not describe the pre-#149 pipeline, which is how #15 became useless. + + ``_BANNED`` already covers the deleted symbols; this asserts the *positive* — that + the document names the ADR-013 mechanisms a correction actually runs through. + """ + text = _CORRECTION.read_text() + offenders = [ + line.strip() + for line in text.splitlines() + if "legacy-ok" not in line and _BANNED.search(line) + ] + assert not offenders, f"the correction procedure describes deleted code: {offenders}" + for mechanism in ("supersed", "commit marker", "source_selection", "C-73"): + assert mechanism in text, ( + f"the procedure does not mention {mechanism!r}. A correction that ignores " + "how the consumer SELECTS a run will publish a fix nobody picks up." + ) + + +def test_the_procedure_states_the_questions_only_the_operator_can_answer(): + """The two external-party decisions must stay visible, not quietly become defaults. + + Who contacts the UN FAO, and whether they expect retraction or supersession. Per + CLAUDE.md both are the operator's; the failure mode is that an undecided step gets + silently improvised the first time it is needed, under time pressure. + """ + text = _CORRECTION.read_text() + assert "not decided" in text.lower(), ( + "the procedure no longer flags its undecided step — if it has been decided, " + "replace the marker with the decision and say who made it" + ) + assert "UN FAO" in text and "supersede" in text