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

[Polichinel]

Advances C-40. No delivered byte changes; no wire module, no manifest, no fixture touched.

The gate C-40 stated was wrong, and that is the main finding

The entry read: "gated on views-pipeline-core 3.0.0, which is a release signal rather than engineering work." 3.0.0 shipped 2026-08-03 and nothing became possible. Two gates had been conflated:

• The pandas gate (consequence b) was gated on pipeline-core's DataFrame retirement — an epic (S5 — The lookup declares its own version instead of inferring it (C-60) #186/Record the operator's correction decisions; put the FAO half to them formally #207), not a version number. It lifted on its own on 2026-07-27 when the frame-native fetch shipped. 3.0.0 had nothing to do with it.
• The de-inheritance was never gated on any pipeline-core release. It is gated on views-models. 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 — removing it means writing a different launcher there, so it is a two-repository change and an operator decision.

Nobody had written that down, which is how the entry sat reading as unblocked work waiting on nobody.

What was actually available

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 building AppwriteConfig and DatastoreModule inline off self.

Now module-level functions with declared arguments: _build_prod_forecasts_store(ensemble_name), _build_partner_store(model_path), _partner_appwrite_config(model_path).

Kept in the same file on purpose. A new module would be a third views_pipeline_core importer and break the allowlist that ADR-015 §2's containment claim rests on. Moving code out of the class gets the testability win without weakening that guarantee.

Why this is worth a PR

tests/test_store_construction.py — 14 tests that could not have been written a day ago. Every one runs with no manager instance, no views-models path manager, and with the Appwrite variables explicitly removed from the environment. That is C-40's consequence (a) — "the FAO logic cannot be instantiated or unit-tested without the full framework + Appwrite env" — discharged for this surface, and the first part of the manager seam reachable without the framework.

They assert the refusals and their ordering: assert_env_declared must run before anything is constructed, so a missing coordinate produces the declared error naming which store is incomplete, rather than whatever the client raises when handed None for an endpoint.

Two things found while in there

• self.ensemble_path_manager — assigned in __init__ and in the 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.
• loa = "pgm" followed by if not loa: raise — an unreachable branch guarding a variable never used again, beside the commented-out block that once computed it for real.

Measurements

	before	after
manager class	351 lines, 16 methods	272 lines, 14 methods
manager file (each partner)	410	440
directory budget	450	450, unchanged
normalised partner diff	prose only	prose only (8 lines, all FAO/CRAF'd naming)

The file grew because the extracted functions carry their reasoning; the class is what C-40 is about.

New ratchet: test_the_manager_class_itself_stays_thin, bound 300. Added because the existing budget counts the directory and therefore cannot see a class re-absorbing logic — precisely the shape of this refactor. Stated in its docstring as a ratchet, not a target: the right response to it failing is to move something out of the class, not to raise the number. Both bounds now stand; neither was relaxed. Adding a constraint while making a change that the existing one is blind to is the honest version of this.

Mutation proofs

mutation	caught by
env assert moved after the config is built	..._checked_before_a_store_is_constructed
a builder given back its self	..._are_functions_not_methods
a coordinate value pasted in	..._no_coordinate_value_is_baked_into_the_builders
the class padded past 300	..._class_itself_stays_thin

Residual, stated honestly

The double inheritance itself, and consequences (c) SDP exposure and (d). Not unilaterally actionable, and arguably not a defect — C-40 has always conceded "the dependency itself is correct — unfao.py genuinely is a pipeline-core postprocessor", and ADR-015 §1 argues there is nothing to unwind to. Tier held at 2: containment is not removal.

Trigger corrected from "a pipeline-core release" to: when views-models changes how postprocessors are launched, 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.

Records disposed of (ADR-014 §5)

C-33's and C-40's Location rows named the old symbols. The dated ADR-011 assessment now banners that its §8 describes the .env borrow and ensemble_path_manager as current, and both are gone.

Verification

ruff check .        # clean
pytest -q           # 377 passed, 40 xfailed

🤖 Generated with Claude Code