From c1f180da1ad266e681333ca623c2db52ea986f85 Mon Sep 17 00:00:00 2001 From: Polichinl Date: Sun, 2 Aug 2026 06:40:50 +0200 Subject: [PATCH] =?UTF-8?q?docs(ops):=20S8=20#189=20=E2=80=94=20a=20correc?= =?UTF-8?q?tion=20procedure=20for=20the=20delivery=20that=20exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C-22's trigger fired on 2026-07-27. Run-0 put 64,742 cells x 36 months into the UN FAO's store, live, and the written procedure (#15, June) described disk caches and shapefiles — both deleted with the runtime mapper. There was data in a partner's store and no usable way to correct it. Affected deliveries are identified by run_id and lookup_version, both present by construction. That is why this story waited on S5: until C-60, lookup_version could silently be the string "unknown", and a procedure whose identification step rests on a field that can become a placeholder is not a procedure. The wire mechanism is supersession, not retraction, and the document says so rather than inventing one — manifest-last commit ordering means a run is replaced by publishing a new complete run. It also states the consequence a reader would otherwise find 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 partial correction published to production is indistinguishable from the real one. That is C-73, cited rather than re-solved. Confirmation is offline against committed artifacts, because re-running the pipeline to investigate changes the thing being investigated. And if the fidelity suite passes while a value is still wrong, the fault is upstream — C-43's scope split, restated where an operator will meet it at 22:00. One step is written but NOT decided, and it is the step that reaches the partner: who contacts the UN FAO and whether they expect retraction or supersession. Both belong to the operator. The document states them verbatim and tells the reader to stop and ask rather than improvise — an inconsistent first message to a partner is harder to correct than a slow one. C-22 closes because the procedure exists and says exactly where it stops. What remains is a decision, not engineering, and leaving the entry open would read as unfinished work rather than as a question awaiting an answer. docs/CLONING.md carries it forward: answer your partner's correction questions BEFORE first delivery. This repo did not. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 6 + docs/CLONING.md | 11 ++ docs/operations/correction_procedure.md | 164 ++++++++++++++++++++++++ reports/technical_risk_register.md | 57 +++++--- tests/test_doc_accuracy.py | 60 +++++++++ 5 files changed, 278 insertions(+), 20 deletions(-) create mode 100644 docs/operations/correction_procedure.md 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