S11 (#197) — living docs described code #148 deleted; the doc guard's ban list was never updated

README.md

@@ -72,11 +72,14 @@ In practice the manager is constructed and run by **views-models**
 
 | Stage | Method(s) | What happens |
 |-------|-----------|--------------|
-| **Read** | `_read_historical_data`, `_read_forecast_data` | Historical actuals from views-datafactory (via the inherited loader); the forecast file from the Appwrite prediction store. The forecast file's identity is checked before use (C-25). |
-| **Transform** | `_transform` → `_append_metadata` | Joins the 9 GAUL metadata columns onto each frame via `GaulLookupEnricher` (a parquet lookup). Prediction values are **not** transformed. |
-| **Validate** | `_validate`, `_check_coverage` | Null-gate on the metadata columns; region coverage + GAUL-excluded-cell guards (C-34 / C-30). |
-| **Clip** | `_clip_observed_history` | Drops fabricated zero-padded tail months from the historical actuals (C-26); the forecast is untouched. |
-| **Save** | `_save` | Writes parquet and uploads to the UN FAO bucket with structured provenance (C-15). |
+| **Read** | `_read_historical_frame`, `_read_forecast_data_contract` | Historical actuals from views-datafactory arrive **frame-native** (#126); the forecast run is resolved from the Appwrite store by its **run manifest**, with each shard's header verified on load (ADR-013 §4.3). |
+| **Transform** | `_transform` | Resolution only. Prediction values are **not** transformed — no collapse, no reconciliation. |
+| **Validate** | `_validate`, `_check_coverage` | Asserts the read resolved, then enforces the region coverage + GAUL-excluded-cell contract (C-34 / C-30). The metadata null-gate fires later, at artifact build (`contract/historical.assert_metadata_complete`). |
+| **Save** | `_save` → `_save_contract` | Builds the ADR-013 wire — arrow shards, the §5 GAUL sidecar, the historical artifact — commits the run **manifest last**, and stamps each upload with structured provenance (C-15). |
+
+The pandas metadata-join and history-clip stages were retired with the legacy delivery path
+in #149; their rules survive as called invariants under `delivery/`. See the
+[manager README](views_postprocessing/unfao/managers/README.md) for what moved where.
 
 ### Output schema (geographic metadata columns)
 

docs/CICs/UNFAOPostProcessorManager.md

@@ -77,11 +77,11 @@ Assumptions that are not met **must cause failure**, not fallback behavior. **Kn
 - **Null values in required metadata columns:** Raises `ValueError` with null count and affected column name (C-01 resolved — validation active)
 - **Dataset initialization failure:** Raises `ValueError` in `_save()` if datasets are None
 - **Appwrite upload failure:** Propagates exception from `DatastoreModule`
-- **Wrong forecast selected:** structurally impossible since #149. Selection is by **run manifest** — a commit marker whose contents are hash-verified — not by scanning the bucket for the newest `category="forecast"` upload. Declared identity is additionally checked **per shard header** against the launched ensemble inside `TargetLease.load()` (`contract/wire/source_selection.py:73-81`), so identity comes from the artifact's own content. The metadata-field check this bullet used to describe (`delivery/identity.py`) was retired in #150 and the legacy reader it served in #149; register C-25 is closed as *superseded by mechanism*
+- **Wrong forecast selected:** structurally impossible since #149. Selection is by **run manifest** — a commit marker whose contents are hash-verified — not by scanning the bucket for the newest `category="forecast"` upload. Declared identity is additionally checked **per shard header** against the launched ensemble inside `TargetLease.load()` (`contract/wire/source_selection.py:73-81`), so identity comes from the artifact's own content. The metadata-field check this bullet used to describe (`delivery/identity.py`) was retired in #150 and the legacy reader it served in #149; register C-25 is closed as *superseded by mechanism* <!-- legacy-ok: retirement record -->
 - **Launch config incomplete:** raises `LaunchConfigError` naming the missing key. A launcher that omits `wire_contract` or declares a `data_format` other than `feature_frame` is **refused**, never quietly routed into a fallback (ADR-003, register C-63)
 - **Region coverage mismatch:** Raises `CoverageError` in `_check_coverage()` (called from `_validate()`) if a pinned region's delivered cell count is wrong (S1/C-34) or a GAUL-uncovered excluded cell leaks into the delivery (S4/C-30)
-- **Fabricated historical tail:** `_read_historical_frame()` drops months beyond the producer's `last_valid_month_id` at the read (`_clip_observed_history` was the pandas equivalent, retired with that path in #149) so unobserved zero-padding is not shipped as observed history (S2/C-26); **degrades open** (skips the clip with a WARNING) if the boundary cannot be resolved
-- **Upload provenance:** every upload's `description` carries structured provenance (lookup version, region, expected/actual cell counts, unmapped count) via `_delivery_description()` (S5/C-15)
+- **Fabricated historical tail:** `_read_historical_frame()` drops months beyond the producer's `last_valid_month_id` at the read (`_clip_observed_history` was the pandas equivalent, retired with that path in #149) so unobserved zero-padding is not shipped as observed history (S2/C-26); **degrades open** (skips the clip with a WARNING) if the boundary cannot be resolved <!-- legacy-ok: retirement record -->
+- **Upload provenance:** the historical artifact's `description` carries structured provenance (lookup version, region, expected/actual cell counts, unmapped count) built by `delivery/provenance.py` (`build_provenance` → `compact_description`) via the manager's `_historical_frame_description()` (S5/C-15). The **forecast** side carries no such description: its guarantee is the wire's verified chain — per-shard content hashes recorded in the §4.2 run manifest, header asserts on load, and manifest-last commit ordering. That is identity and integrity, not the C-15 provenance field set; the §4.2 manifest's keys are exactly `contract_version`, `run_id`, `targets`, `shards`, `expected_months`, `expected_cell_count`, `sidecar` — and it carries **no** `lookup_version`, `region` or `unmapped_count`. `_delivery_description()` was the pandas-path equivalent and was deleted with it in #149 <!-- legacy-ok: retirement record -->
 
 The following **must never** fail silently:
 - Missing or None environment variables for Appwrite
@@ -145,7 +145,7 @@ manager._save()
 - **Beige tests:** Missing ensemble name in config; None environment variables; empty forecast bucket; DataFrames with unexpected index structure
 - **Red tests:** Corrupted parquet downloads; network timeouts during upload; DataFrames where all cells map to None (all-ocean input)
 
-Currently: the manager cannot be instantiated without `views-pipeline-core`, so its stage logic is covered by **source-scan and replica tests** — `tests/test_validation.py`, `tests/test_launch_config.py` (the refusals), `tests/test_gaul_lookup_access.py` (one lookup read, threaded). `tests/test_append_metadata.py` was deleted in #149 with the method it mirrored. A full end-to-end test against the live manager (C-03) still requires a production-like environment.
+Currently: the manager cannot be instantiated without `views-pipeline-core`, so its stage logic is covered by **source-scan and replica tests** — `tests/test_validation.py`, `tests/test_launch_config.py` (the refusals), `tests/test_gaul_lookup_access.py` (one lookup read, threaded). `tests/test_append_metadata.py` was deleted in #149 with the method it mirrored. <!-- legacy-ok: retirement record --> A full end-to-end test against the live manager (C-03) still requires a production-like environment.
 
 The input-integrity guards (S0–S6, epic #51) are representation-free invariants in `views_postprocessing/delivery/` that the manager **calls** (never inherits). Each has primitives unit tests — `tests/test_delivery_coverage.py` (S1/S4), `tests/test_delivery_observed_range.py` (S2), `tests/test_provenance.py` (S5), `tests/test_frame_extraction.py` (the seam), `tests/test_store_metadata.py` (store identity) — and `tests/test_input_integrity_e2e.py` drives the invariants on **primitives**, which is how the manager calls them. S3's forecast-identity rule moved to the wire layer (#150) and is covered by `tests/test_wire_source_selection.py`. The design contract (representation-free, called-not-inherited) is pinned by `tests/test_input_integrity_design_contract.py`.
 

docs/architecture/role_and_seams.md

@@ -55,8 +55,9 @@ pattern:
   `_read → _transform → _validate → _save` in order.
 - this repo **fills in the steps** for the FAO path (the `_read*/_transform/_validate/_save`
   overrides in `unfao/managers/unfao.py`).
-- pipeline-core also **provides the tools** the steps use: `ViewsDataLoader`, `PGMDataset`,
+- pipeline-core also **provides the tools** the steps use: `ViewsDataLoader`,
   `DatastoreModule`, `AppwriteConfig`, the path managers.
+  (`PGMDataset` was on this list until #149 retired the pandas delivery path; this repo no longer references it — see Seam B.) <!-- legacy-ok: retirement record -->
 
 So the runtime control flow is *inverted* ("don't call us, we'll call you"): views-models
 calls `manager.execute()`, which lives in **pipeline-core's base**, which calls back into
@@ -110,20 +111,20 @@ The manager **calls** the invariants; it never makes them methods of itself. The
 always `extract (seam) → call invariant → raise`. This is why the guards are testable
 without the framework, and why they survive a representation change untouched.
 
-**And they did survive one.** This seam was `unfao/extraction.py` (pandas) until #151.
+**And they did survive one.** This seam was `unfao/extraction.py` (pandas) until #151. <!-- legacy-ok: retirement record -->
 When the pandas delivery was retired, the invariants needed **no change at all** — only
 which module fed them. That is the design working exactly as this section claims, and it is
-the evidence for the claim rather than a restatement of it. `delivery/identity.py` was also
-retired (#150): the forecast-identity rule now lives in the wire layer, checked per shard
-header against declared provenance — see §5 Seam C.
+the evidence for the claim rather than a restatement of it. `delivery/identity.py` was also <!-- legacy-ok: retirement record -->
+retired (#150): the forecast-identity rule now lives in the wire layer, checked per shard <!-- legacy-ok: retirement record -->
+header against declared provenance — see §5 Seam C. <!-- legacy-ok: retirement record -->
 
 ### Seam B — the inherited pandas base, and the C-40 gate
 
 Because this repo *is-a* pipeline-core postprocessor (section 3), three **concrete** pandas
 pieces are inherited, not chosen:
 
 1. the input loader (`ViewsDataLoader` → parquet → pandas),
-2. the dataset container (`PGMDataset`, a pandas `DataFrame` with object-dtype cells),
+2. the dataset container (`PGMDataset`, a pandas `DataFrame` with object-dtype cells), <!-- legacy-ok: describes the pre-2026-07-27 state, as this section states -->
 3. the prediction-store parquet I/O.
 
 **This section described the state until 2026-07-27; it is no longer true and is kept
@@ -147,7 +148,7 @@ samples per cell). This is where representation matters most:
 
 - views-frames stores a distribution natively as a contiguous `(N, S)` float32 array (sample
   axis explicit; a point is just `S=1`).
-- pandas `PGMDataset` stores it as **object-dtype list-in-cell** — a separate numpy array
+- pandas `PGMDataset` stores it as **object-dtype list-in-cell** — a separate numpy array <!-- legacy-ok: comparative reasoning for the frame-native path -->
   boxed in each of N cells. Cost scales ~linearly with S (memory, an encode/decode tax at
   every parquet/API boundary, a silent resize on mismatched sample counts).
 

tests/test_doc_accuracy.py

@@ -24,10 +24,36 @@
 
 # --- 1. deleted-symbol scan -------------------------------------------------------------
 
-# Symbols that name code removed in ADR-011 / C-39. None should appear as *current* in a
-# living onboarding doc.
+# Symbols naming code this repo has deleted. None should appear as *current* in a living
+# onboarding doc.
+#
+# **This list must grow with every deletion, and that is the lesson of S11 (#197).** It
+# carried only the ADR-011 / C-39 entries for six weeks while epic #148 deleted a whole
+# delivery path (#149) and moved the machinery out of `unfao/` (#153). The guard kept
+# passing over four documents describing methods and files that no longer existed —
+# including `unfao/managers/README.md`, which documented the Template Method as
+# `_transform → _append_metadata` and pointed readers at a deleted test file, sitting
+# inside the package next to the code it misdescribed.
+#
+# The guard was not wrong. It was not updated. A deletion PR that does not extend this
+# regex has not finished.
 _BANNED = re.compile(
-    r"PriogridCountryMapper|mapping/README|use_disk_cache|cachetools|geopandas",
+    "|".join((
+        # ADR-011 / C-39 — the runtime mapper and its shapefile toolchain (June 2026)
+        r"PriogridCountryMapper", r"mapping/README", r"use_disk_cache", r"cachetools",
+        r"geopandas",
+        # #149 — methods of the retired pandas delivery path (2026-07-31)
+        r"_append_metadata", r"_delivery_description", r"_clip_observed_history",
+        r"LEGACY_FORECAST_FILTERS", r"test_append_metadata",
+        # #150 / #151 / #153 — modules retired or moved out of `unfao/` by epic #148
+        r"delivery/identity", r"unfao/extraction", r"unfao/frames",
+        r"unfao/historical", r"unfao/gaul_schema", r"unfao/wire",
+        # #149 — pipeline-core's pandas container, no longer referenced by this repo.
+        # Negative lookbehind: `FAO_PGMDataset` is views-faoapi's class and is live.
+        # It inherits IGNORECASE, so `fao_PGMDataset` is spared too — no such spelling
+        # exists, but the lookbehind is not case-exact and a reader should not assume it.
+        r"(?<!FAO_)PGMDataset",
+    )),
     re.IGNORECASE,
 )
 
@@ -40,14 +66,29 @@ def _living_docs() -> list[Path]:
     return [d for d in docs if d.exists()]
 
 
-def test_living_docs_have_no_deleted_symbol_references():
+def _label(doc: Path) -> str:
+    """Repo-relative path where possible — three living docs are named ``README.md``,
+    so a bare basename cannot say which one failed."""
+    try:
+        return doc.relative_to(_REPO).as_posix()
+    except ValueError:
+        return doc.name  # a doc injected from tmp_path by the guard's own tests
+
+
+def _deleted_symbol_offenders(docs: list[Path]) -> list[str]:
+    """Scan ``docs`` for banned symbols, honouring the line-scoped ``legacy-ok`` opt-out."""
     offenders = []
-    for doc in _living_docs():
+    for doc in docs:
         for i, line in enumerate(doc.read_text().splitlines(), start=1):
             if "legacy-ok" in line:  # explicit opt-out for an intentional historical mention
                 continue
             if _BANNED.search(line):
-                offenders.append(f"{doc.relative_to(_REPO)}:{i}: {line.strip()}")
+                offenders.append(f"{_label(doc)}:{i}: {line.strip()}")
+    return offenders
+
+
+def test_living_docs_have_no_deleted_symbol_references():
+    offenders = _deleted_symbol_offenders(_living_docs())
     assert not offenders, "deleted-symbol references in living docs:\n" + "\n".join(offenders)
 
 
@@ -193,3 +234,65 @@ def test_the_retired_contract_name_is_gone_from_code():
         "pinned URL (views-appwrite/docs/ADRs/platform/coordinate_registry.toml). "
         "Historical narration belongs in docs/ADRs or reports/, not here."
     )
+
+
+def test_the_ban_covers_the_post_148_deletions_and_spares_the_live_lookalike():
+    """A ban list that was not updated when the code changed is why S11 (#197) existed.
+
+    Two failure modes, both real, so both are pinned:
+
+    1. **Entries going missing** — someone trims the list and a document quietly starts
+       describing deleted code again. The loop below fails if any known entry stops
+       matching. Note what it *cannot* catch: a future deletion whose symbol nobody adds
+       is invisible to this test and to the guard alike, which is why the comment above
+       ``_BANNED`` says a deletion PR that does not extend it has not finished. No test
+       can substitute for that; only the habit can.
+    2. **Too broad** — `FAO_PGMDataset` is a *views-faoapi* class, alive and correctly
+       cited in `contract/gaul_schema.py` and `contract/enrichment.py`. A careless
+       `PGMDataset` pattern would ban a true statement about another repo's code.
+    """
+    for deleted in (
+        "_append_metadata", "_delivery_description", "_clip_observed_history",
+        "LEGACY_FORECAST_FILTERS", "tests/test_append_metadata.py",
+        "delivery/identity.py", "unfao/extraction.py", "unfao/frames.py",
+        "unfao/historical.py", "unfao/gaul_schema.py", "unfao/wire/sink.py",
+        "PGMDataset", "PriogridCountryMapper", "geopandas",
+    ):
+        assert _BANNED.search(deleted), (
+            f"{deleted!r} names code this repo deleted, but the ban list does not cover "
+            "it — a living doc could describe it as current and nothing would object"
+        )
+
+    assert not _BANNED.search("FAO_PGMDataset._METADATA_COLS"), (
+        "the ban caught `FAO_PGMDataset`, which is views-faoapi's live class and the "
+        "consumer contract our sidecar is written against — banning a true statement "
+        "about another repo's code is how a guard earns its own deletion"
+    )
+
+
+def test_the_ban_actually_fires_on_a_living_doc(tmp_path):
+    """The scan, not just the regex — a pattern nothing applies is decoration."""
+    doc = tmp_path / "README.md"
+    doc.write_text("The manager joins metadata in `_append_metadata`.\n")
+    assert _deleted_symbol_offenders([doc]), (
+        "the scan reported nothing on a doc naming a deleted method"
+    )
+
+
+def test_the_legacy_ok_marker_still_works(tmp_path):
+    """The escape hatch the retirement records depend on — line-scoped, deliberately.
+
+    Four documents now name deleted methods in order to say where the work went. That is
+    better than a gap, and it only stays possible while this marker keeps working. It is
+    line-scoped: a marker on the following line does not excuse the mention above it.
+    """
+    excused = tmp_path / "ok.md"
+    excused.write_text("`_append_metadata` was retired in #149. <!-- legacy-ok: retirement record -->\n")
+    assert not _deleted_symbol_offenders([excused])
+
+    wrong_line = tmp_path / "wrong.md"
+    wrong_line.write_text("`_append_metadata` was retired.\n<!-- legacy-ok -->\n")
+    assert _deleted_symbol_offenders([wrong_line]), (
+        "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"
+    )