refactor(managers): C-40 — store construction off the manager class

docs/ADR-011_implementation_assessment.md

@@ -1,6 +1,14 @@
 # ADR-011 Implementation Assessment
 
 **Last updated:** 2026-06-12
+> **This is a dated snapshot (2026-06-12), not a description of the code today.** It is
+> kept as a record of what was known when ADR-011 was assessed. Several things it
+> describes as current have since been removed — notably the `.env` credential borrow
+> (killed by þing-01 #134; the environment is now declared and validated fail-loud in
+> each partner's `appwrite_env.py`) and `self.ensemble_path_manager`, which stopped
+> existing on 2026-08-05 when store construction moved off the manager class (register
+> C-40). Read §8 in particular as history.
+
 **Status:** Data prerequisites MET — see §10. Implementation unblocked on the data side; verification infrastructure (Appwrite, pipeline-core E2E) still required before switching the pipeline.
 
 ---

reports/technical_risk_register.md

@@ -345,7 +345,7 @@ See also D-10 (handling decision), C-43 (the *value*-correctness sibling — run
 | Source | `expert-code-review` (2026-06-12) |
 | Trigger | **Fired 2026-08-03 — see the update below.** The remaining trigger is the *extraction* one, and it is now named: a **third** in-repo partner package, **or** the first bug that must be hand-patched identically in both manager files — whichever comes first. |
 | Owner | Whoever adds the third partner package, or hits the first double-patch. Until one of those happens the duplication is the deliberate WET position, not a task anyone is behind on. |
-| Location | `views_postprocessing/<partner>/managers/<partner>.py` — `_prod_forecasts_datastore`, `_<partner>_datastore`, `_<partner>_appwrite_config`, and the four hardcoded `os.getenv("APPWRITE_<PARTNER>_*")` literals inside the last of those; declared names in each partner's `appwrite_env.py`. **Symbols, not line numbers** — see the note under the measurement below. |
+| Location | `views_postprocessing/<partner>/managers/<partner>.py` — the module-level `_build_prod_forecasts_store`, `_build_partner_store`, `_partner_appwrite_config` (methods until 2026-08-05), and the four hardcoded `os.getenv("APPWRITE_<PARTNER>_*")` names inside the last of those; declared names in each partner's `appwrite_env.py`. **Symbols, not line numbers** — see the note under the measurement below. |
 
 Mitigation: a small `DeliveryProfile` (bucket/collection/database ids, category, targets) passed to the manager — one manager class, N store configs. Scheduled **after** the FAO global delivery ships (D-09); the only immediate action is deleting the commented-out config blocks at lines 80-107, which are a mis-uncomment hazard during deadline work.
 
@@ -398,7 +398,7 @@ See also C-24 (schema contract per store), C-77 (the fourth home for partner ide
 | Tier | 2 |
 | Source | `expert-code-review` (2026-06-24) |
 | Trigger | **(a) Upstream change:** when pipeline-core changes `PGMDataset` / the data loader / the postprocessor base (mid-migration: their #186/#188/#161), verify the inherited surface this repo depends on still holds. **(b) Standing work item:** the input-side de-inheritance (the sink side landed — see the 2026-07-31 update) — schedule it, don't wait for a trigger. |
-| Location | `views_postprocessing/<partner>/managers/<partner>.py` — the `class <PARTNER>PostProcessorManager(PostprocessorManager, ForecastingModelManager)` statement (double inheritance); `_prod_forecasts_datastore`, `_<partner>_datastore`, `_<partner>_appwrite_config` (inline env/AppwriteConfig/DatastoreModule); `_validate` and `_check_coverage`; the DIP sink adapter `_ContractStorePort`. **Since 2026-08-03 all of it exists twice** — `unfao` and `crafd` are the same file with the partner name changed (C-33). Symbols rather than lines, deliberately: an earlier version of this row was invalidated by a comment edit four lines long. |
+| Location | `views_postprocessing/<partner>/managers/<partner>.py` — the `class <PARTNER>PostProcessorManager(PostprocessorManager, ForecastingModelManager)` statement (double inheritance); `_build_prod_forecasts_store`, `_build_partner_store`, `_partner_appwrite_config` (env/AppwriteConfig/DatastoreModule — moved OFF the class 2026-08-05, see the update below); `_validate` and `_check_coverage`; the DIP sink adapter `_ContractStorePort`. **Since 2026-08-03 all of it exists twice** — `unfao` and `crafd` are the same file with the partner name changed (C-33). Symbols rather than lines, deliberately: an earlier version of this row was invalidated by a comment edit four lines long. |
 
 `UNFAOPostProcessorManager` subclasses **two concrete** pipeline-core base classes (`PostprocessorManager`, `ForecastingModelManager`) and **interleaves infrastructure** (env reading, `AppwriteConfig` construction, `DatastoreModule`, path resolution) with the FAO **business logic** (GAUL enrichment, the 9-column null gate) inside the lifecycle hooks. Consequences: (a) the FAO logic cannot be instantiated or unit-tested without the full framework + Appwrite env + viewser; (b) **pandas cannot leave the delivery path** because the inherited data loader and `PGMDataset` are pandas — gated on pipeline-core's own DataFrame retirement; (c) **SDP exposure** — heavy *inheritance* coupling to a pipeline-core that is itself unstable (mid-migration), so upstream changes break far from their cause (cf. C-27, C-29); (d) it's the repo's only composition-over-inheritance violation. The dependency itself is correct (`unfao.py` genuinely *is* a pipeline-core postprocessor) — the issue is its **blast radius**. Mitigation (does **not** fight the Template-Method framework): keep the subclass as a **thin shell** but extract `enrich` + `validate` + the 9-column contract into a pipeline-core-free core object the manager *calls*, and wrap the Appwrite I/O behind a small delivery-sink adapter (DIP). This makes the FAO logic testable standalone and insulates it from pipeline-core churn.
 
@@ -436,7 +436,26 @@ See also C-24 (schema contract per store), C-77 (the fourth home for partner ide
 
 **On þing-02 S24(5).** `docs/CLONING.md` cited that verdict as forbidding these imports outright. Reading it directly (`þingit/02_credential_identity_key_ownership/sáttmál.md:240-242` — precondition (5) itself; the section opens at `:232` under the heading *"§5 — The clone (`un-crafdapi`)"* — and `orð_dómr.md:418-441`), it binds *"the clone"* — `un-crafdapi` and `views-productionapi`, repositories **git-cloned from views-faoapi** — and does not reach an in-repo partner package of the producer. CLONING.md over-claimed; PR #211 corrects the citation rather than weakening the rule. This entry's own scope is unaffected: the coupling is a design concern here regardless of what the verdict binds, and issue **#146**'s deferred unwind now covers two files instead of one.
 
-Tier held at 2. The residual scope — the double inheritance and the framework-bound instantiation — is unchanged, and is still gated on views-pipeline-core 3.0.0 (C-44/C-62).
+**Update 2026-08-05 — the stated gate was wrong, and the source half of the mitigation has now landed.**
+
+*The gate first, because it is what kept this entry parked.* The line below said the residual was *"gated on views-pipeline-core 3.0.0, which is a release signal rather than engineering work"*. 3.0.0 shipped on 2026-08-03 and nothing became possible. Two different gates had been conflated:
+
+- **Consequence (b), the pandas gate** — genuinely gated on pipeline-core's DataFrame retirement, which is an epic (#186/#207), not a version number. It lifted on its own on 2026-07-27 when the frame-native fetch shipped, and 3.0.0 had nothing to do with it.
+- **The de-inheritance** — never gated on any pipeline-core release. **It is gated on views-models**, which is the thing nobody had written down. `postprocessors/un_fao/main.py:27` constructs `UNFAOPostProcessorManager(...)` directly, and the framework's Template Method drives `_read`/`_transform`/`_validate`/`_save`. **The inheritance *is* the integration contract with views-models.** Removing it means writing a different launcher there, so it is a two-repository change and an operator decision — not, as this entry implied for two days, unblocked work waiting on nobody.
+
+*What landed.* This entry's own prescribed mitigation was to wrap the Appwrite I/O behind a DIP adapter. The **outbound** half landed in July as `_ContractStorePort`. The **inbound** half never did: `_prod_forecasts_datastore`, `_<partner>_datastore` and `_<partner>_appwrite_config` were still methods on the manager, constructing `AppwriteConfig` and `DatastoreModule` inline. They are now module-level functions taking declared arguments — `_build_prod_forecasts_store`, `_build_partner_store`, `_partner_appwrite_config`.
+
+That is a smaller change than it sounds and a bigger one than it looks. Smaller: no delivered byte moves, and the three functions are the same code with their inputs declared. Bigger: **consequence (a) is substantially discharged for this surface.** Store construction, its refusals and their ordering are now testable with no manager instance, no views-models path manager and no Appwrite environment — `tests/test_store_construction.py`, 14 tests, which could not have been written at any price a day ago. That is the first time any part of the manager seam has been reachable without the framework.
+
+*Two things found while in there.* `self.ensemble_path_manager` was assigned in `__init__` and in the store builder, read exactly once four lines after being set, and read nowhere else in either partner package or in views-models — a method-local value wearing the costume of manager state. And a `loa = "pgm"` assignment followed by `if not loa: raise`, an unreachable branch guarding a variable never used again, beside a commented-out block that had computed it for real. Both gone.
+
+*Measured.* The manager **class** went 351 → **272 lines** (16 → 14 methods); both partner files are 440 lines against a 450 directory budget. A new ratchet, `test_the_manager_class_itself_stays_thin`, bounds the class at 300 — added because the existing budget counts the *directory*, so it cannot see a class re-absorbing logic, and this refactor is precisely the shape it is blind to. Both bounds now stand; neither was relaxed.
+
+**Residual scope, stated honestly.** The double inheritance itself, and with it consequences (c) SDP exposure and (d) composition-over-inheritance. Not unilaterally actionable, and arguably not a defect: this entry has always conceded that *"the dependency itself is correct — `unfao.py` genuinely **is** a pipeline-core postprocessor"*, and ADR-015 §1 argues there is nothing to unwind to. What remains is exposure to an upstream that moves, which 3.0.0 demonstrated by moving.
+
+**Trigger, corrected.** Not a pipeline-core release. Revisit when **views-models changes how postprocessors are launched** — at which point de-inheritance becomes a one-repo change rather than two — or when an upstream base-class change actually breaks a delivery, which `tests/test_framework_contract.py` now catches here rather than on the partner's run.
+
+Tier held at 2: the blast-radius argument is unchanged for what remains, and containment is not removal.
 
 *Did not:* the double inheritance (the `class UNFAOPostProcessorManager(...)` statement — this row cited `unfao.py:80` when written, and that number has moved twice since) stands, and so do consequences (a) — the FAO logic still cannot be instantiated without the framework — and (c)/(d). **This entry remains open on exactly that scope.** Its remaining fix is gated on views-pipeline-core's 3.0.0 (C-44/C-62), which is a release signal rather than engineering work.
 

tests/test_doc_accuracy.py

@@ -367,6 +367,10 @@ def test_internal_doc_links_resolve():
 #: previously unbudgeted, which is the same regrowth wearing a different filename.
 _MANAGER_LINE_BUDGET = 450
 
+#: The manager CLASS, separately (C-40). 351 before the 2026-08-05 extraction, 272 after.
+#: A ratchet — see `test_the_manager_class_itself_stays_thin` for why it is not a target.
+_MANAGER_CLASS_BUDGET = 300
+
 
 def _is_type_checking(test: ast.expr) -> bool:
     """`TYPE_CHECKING` or `typing.TYPE_CHECKING`, and nothing else.
@@ -488,6 +492,37 @@ def test_the_manager_stays_within_its_line_budget(managers_dir):
     )
 
 
+@pytest.mark.parametrize("partner", _PARTNER_PACKAGES)
+def test_the_manager_class_itself_stays_thin(partner):
+    """The directory budget above is anti-regrowth. This one is anti-*fusion*.
+
+    They measure different things and both are needed. A file can sit comfortably under
+    450 lines while the manager **class** absorbs store construction, env assembly and
+    business logic — which is exactly what register C-40 is about, and exactly what the
+    directory count cannot see. Conversely a thin class proves nothing if 800 lines
+    moved to a sibling module, which is why the directory count exists.
+
+    Added 2026-08-05 alongside the C-40 extraction that took the class from 351 lines to
+    272. The bound is a **ratchet, not a target**: it is the measured value plus modest
+    headroom, and the right response to it failing is to move something out of the class,
+    not to raise the number. If it is ever raised, the raise belongs in a commit message
+    that says what was added and why it had to live on the manager.
+    """
+    source = (_PKG / partner / "managers" / f"{partner}.py").read_text()
+    cls = next(
+        n for n in ast.walk(ast.parse(source))
+        if isinstance(n, ast.ClassDef) and n.name.endswith("PostProcessorManager")
+    )
+    lines = cls.end_lineno - cls.lineno + 1
+    assert lines <= _MANAGER_CLASS_BUDGET, (
+        f"{cls.name} is {lines} lines, over the {_MANAGER_CLASS_BUDGET} ratchet. The "
+        "manager is the Template-Method shell: it orchestrates hooks and adapts the "
+        "framework. Logic that can be called with declared arguments belongs in a "
+        "module-level function or in contract/ — where it can be tested without a "
+        "manager, an Appwrite environment or a views-models path manager (C-40 (a))."
+    )
+
+
 def _imported_subpackages(source: str, module_path: Path) -> set[str]:
     """Every ``views_postprocessing.<X>`` this module imports — all four spellings.