diff --git a/.github/workflows/publish_package.yml b/.github/workflows/publish_package.yml index 949525f..b78c50c 100644 --- a/.github/workflows/publish_package.yml +++ b/.github/workflows/publish_package.yml @@ -4,14 +4,17 @@ name: Publish Package # (create a Release from a vX.Y.Z tag). Auth is PyPI Trusted Publishing (OIDC) — # no stored API token. See docs/guides/publishing-to-pypi.md. # -# One-time PyPI setup (project Settings -> Publishing -> add a trusted publisher, -# or a *pending* publisher for the first-ever release before the project exists): +# One-time PyPI setup (already done for this project — kept for reference when +# bootstrapping a NEW package: a *pending* publisher covers the first-ever release): # Publisher: GitHub | Owner: views-platform | Repo: views-baseline # Workflow: publish_package.yml | Environment: (leave blank) # -# NOTE: views-baseline depends on views-pipeline-core (>=3.0.0), which is not yet -# on PyPI — `pip install views-baseline` will not resolve until that dependency is -# published. Uploading here still succeeds and reserves the name/version. +# NOTE: views-pipeline-core (>=3.0.0) reached PyPI on 2026-08-03 and is now at 3.2.0, +# so `pip install views-baseline` resolves. 1.0.0 and 1.0.1 were published while it did +# not, and were installable in name only. 1.0.2 is therefore the FIRST release whose +# clean-room install-back can actually be exercised — do it (docs/guides/publishing-to-pypi.md +# §A). This note said the opposite until 2026-09-09; it was the third copy of that claim +# and the one the doc sweep missed. on: release: diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index fc9bda5..ed171cb 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -2,10 +2,22 @@ name: Lint and Test # uv-based CI (mirrors the views-frames / views-datafactory / views-reporting pattern). # -# NOTE: `uv sync` cannot currently resolve the dependency graph because -# views-pipeline-core (>=3.0.0) is not on PyPI yet, so this workflow is expected to -# be red until that dependency is published. The editable `views_pipeline` conda env -# is the working test gate meanwhile (README / CONTRIBUTING). +# This workflow RUNS and passes again as of 2026-09-09 (issue #90). It is NOT yet a +# blocking gate: neither `main` nor `development` declares required status checks +# (verified via the branch-protection API), so a red run here still does not stop a +# merge. Turning these into required checks is a repository-settings change for the +# maintainer — worth doing now that the job is reliably green, and tracked in C-49. +# If you enable it, use the CURRENT job name `check (3.11)`; the pre-2026-09-09 runs +# report `check (3.12)`/`check (3.13)`, which no longer exist and would block every PR. +# +# History, so the next person does not re-diagnose it: it was red from 2026-08-02 and the +# note here blamed views-pipeline-core not being on PyPI. pipeline-core 3.0.0 published on +# 2026-08-03 and the workflow was never re-run, so the note went stale and the real cause +# went unseen for five weeks — which is also how the #84 outage survived, since nothing was +# gating merges. The actual blocker was different and would have outlived the publish: +# uv resolves a universal lockfile including win32, and +# pipeline-core -> viewser -> docker<6 -> pywin32==227 has no wheels above cp39. Fixed by +# `[tool.uv] environments = ["sys_platform == 'linux'"]` in pyproject.toml. on: push: @@ -19,9 +31,8 @@ on: workflow_dispatch: jobs: - # Lint runs project-independently (uvx), so it stays a reliable green signal even - # while `uv sync` cannot resolve views-pipeline-core (see the header note) — the - # `check` job below is gated behind that resolution and is expected red until then. + # Lint runs project-independently (uvx), so it does not depend on the project + # resolving. Kept separate for that reason, though `check` now resolves too. lint: runs-on: ubuntu-latest steps: @@ -34,7 +45,23 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.11", "3.12", "3.13"] + # 3.11 only, and not by preference (issue #95). + # + # views-pipeline-core -> viewser 6.6.4 -> `pandas<2.0.0,>=1.4.0`, and pandas 1.5.3 + # (the last 1.x) publishes wheels for cp38-cp311 only. On 3.12/3.13 the resolver + # must build pandas from source, which fails outright: + # ModuleNotFoundError: No module named 'pkg_resources' + # So views-baseline is not installable on 3.12/3.13 at all — a transitive + # constraint two hops upstream, not something this repo can fix, and true long + # before this matrix was written. + # + # `pyproject.toml` still declares `requires-python = ">=3.11,<3.14"` and 3.12/3.13 + # classifiers. That claim is currently false; it is tracked in the register (C-47) + # rather than quietly narrowed here, because what the package claims to support is + # a distribution decision, not a CI one. + # + # Restore the full matrix when viewser drops the `pandas<2` pin. + python-version: ["3.11"] steps: - uses: actions/checkout@v4 - uses: astral-sh/setup-uv@v5 diff --git a/README.md b/README.md index 77c3030..cb60d2d 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,7 @@ Key characteristics: Input DataFrame: * Must be indexed by `(time, entity)` as a MultiIndex -* Must contain target columns specified in `config['targets']` +* Must contain target columns specified in `config['regression_targets']` (the `targets` key was retired upstream in views-pipeline-core #380 and is now *rejected* — do not add it back) Example index: diff --git a/docs/ADRs/009_boundary_contracts_and_configuration_validation.md b/docs/ADRs/009_boundary_contracts_and_configuration_validation.md index 99c91de..c5a779c 100644 --- a/docs/ADRs/009_boundary_contracts_and_configuration_validation.md +++ b/docs/ADRs/009_boundary_contracts_and_configuration_validation.md @@ -36,12 +36,16 @@ Each module boundary is documented with an explicit contract. Where validation e `MODEL_GENOMES` encodes the required keys per model: ```python -MODEL_GENOMES = { - "ZeroModel": [], - "LocfModel": [], - "AverageModel": ["window_months"], - "ConflictologyModel": ["window_months", "n_samples"], - "MixtureBaseline": ["window_months", "lambda_mix", "n_samples"], +CORE_GENOME = ["steps", "time_steps", "prediction_format", "regression_targets", "level"] + +MODEL_GENOMES = { # alias of ReproducibilityGate.Config.ALGORITHM_GENOMES + "ZeroModel": [], + "LocfModel": [], + "AverageModel": ["window_months"], + "ConflictologyModel": ["window_months", "n_samples", "seed"], + "MixtureBaseline": ["window_months", "lambda_mix", "n_samples", "seed"], + "ParametricConflictology": ["window_months", "n_samples", "seed", "family", "transform"], + "ParametricHurdleConflictology": ["window_months", "n_samples", "seed", "family", "transform"], } ``` @@ -50,7 +54,7 @@ All model-specific parameters are required and must be explicitly declared in th **What is not validated:** - Value ranges: `window_months=0`, `window_months=-1`, `lambda_mix=2.0`, `n_samples=0` all pass validation. -- `targets` is required by all models but is not listed in `MODEL_GENOMES` (it is assumed to always be present). If `targets` is missing from config, construction of the model object will raise a `KeyError` from within the factory method, not from the validation check. +- ~~`targets` is required by all models but is not listed in `MODEL_GENOMES`~~ — **closed 2026-09-09 (issue #85).** This bullet is retained rather than deleted, because what it recorded is worth remembering: the gap was written down as accepted debt in March 2026, and it is exactly what the August outage was made of. views-pipeline-core retired the synthesised `targets` key on 2026-08-02 (#380); the catalog kept reading it; every baseline model raised the predicted `KeyError` from inside the factory method for five weeks, with the audit reporting success one line earlier. The catalog now reads `regression_targets`, and both it and `level` are declared in `CORE_GENOME`, so absence is a `MissingHyperparameterError` naming the key at the boundary. **Accepted debt in a boundary contract is a prediction, not an observation.** - `partition_dict` is passed directly to model constructors without any structural check. The expected structure is `{"test": (start, end)}` where `start` and `end` are integer month IDs, but this is not enforced. --- @@ -63,7 +67,7 @@ All model-specific parameters are required and must be explicitly declared in th Each factory method maps config keys to constructor parameters. The mapping is explicit and readable: -- `config["targets"]` → `targets=` +- `config["regression_targets"]` → `targets=` (derived **once** in `__init__` as `self.targets`, not re-read at each of the seven factory sites, so a future rename of the platform's target vocabulary is one edit rather than seven) - `config["window_months"]` → `window_months=` (for `AverageModel`, `ConflictologyModel`, and `MixtureBaseline`) - `config["lambda_mix"]` → `lambda_mix=` (for `MixtureBaseline`) - `config["n_samples"]` → `n_samples=` (for `ConflictologyModel` and `MixtureBaseline`) @@ -170,7 +174,7 @@ from views_pipeline_core.data.prediction_frame import PredictionFrame | Boundary | What is missing | |---|---| -| Config → Catalog | No validation that `targets` is present; no value-range validation for numeric params | +| Config → Catalog | No value-range validation for numeric params. (Target/level presence: **closed** — `regression_targets` and `level` are in `CORE_GENOME` since #85.) | | Catalog → Model | No constructor-level range checks; no type coercion | | Model → Pipeline | No validation of distributional output structure or shape | | Data → Model | No MultiIndex structure validation; no column presence check; no time-range sanity check | diff --git a/docs/ADRs/012_model_addition_protocol.md b/docs/ADRs/012_model_addition_protocol.md index aaea07c..331ef76 100644 --- a/docs/ADRs/012_model_addition_protocol.md +++ b/docs/ADRs/012_model_addition_protocol.md @@ -38,7 +38,7 @@ This ADR serves as the authoritative checklist for adding a new model. **File:** `views_baseline/model/catalog.py` 1. Add an import for the new class at the top of the file. -2. Add an entry to `MODEL_GENOMES` listing required config keys (beyond `targets`). For a **stochastic** model this MUST include `seed` — it is a required, audited genome key with no magic default (ADR-021). For a **parametric-family** model (ADR-022) it MUST also include `family` and `transform`; the constructor is responsible for failing loud on an unsupported family or an illegal `family×transform` combination. +2. Add an entry to `MODEL_GENOMES` listing required config keys (beyond the `CORE_GENOME` keys, which now include `regression_targets` and `level`). For a **stochastic** model this MUST include `seed` — it is a required, audited genome key with no magic default (ADR-021). For a **parametric-family** model (ADR-022) it MUST also include `family` and `transform`; the constructor is responsible for failing loud on an unsupported family or an illegal `family×transform` combination. 3. Add a factory method `_get_()` that reads config and constructs the model. 4. Add the model name → factory method mapping in `self.models` inside `__init__`. diff --git a/docs/CICs/BaselineForecastingModelManager.md b/docs/CICs/BaselineForecastingModelManager.md index d5e09c1..30a7b38 100644 --- a/docs/CICs/BaselineForecastingModelManager.md +++ b/docs/CICs/BaselineForecastingModelManager.md @@ -73,7 +73,7 @@ All models now return `dict[str, PredictionFrame]` from `predict()`. The manager | Source | What is consumed | Notes | |---|---|---| | `model_path` (constructor) | `ModelPathManager` instance | Provides `.artifacts` path. `.data_raw` is no longer accessed directly (data path comes from base class `_cached_data_path`). | -| `self.config` | `dict` | Set via property inherited from base class. Must contain `"run_type"`, `"level"`, `"algorithm"`, `"targets"`. | +| `self.config` | `dict` | Set via property inherited from base class. Must contain `"run_type"`, `"algorithm"`, and every `CORE_GENOME` key — `"steps"`, `"time_steps"`, `"prediction_format"`, `"regression_targets"`, `"level"` — all audited by `audit_manifest` before the catalog is built. (`targets` was retired upstream in pipeline-core #380; see #85. `run_type` is read one line after the audit but is NOT audited — it is injected at runtime, not declared in a config file; see C-48.) | | `_data_loader.partition_dict` | `dict` | Must contain `"test"` key. | | `_config_manager` | `ConfigurationManager` | Used by base class and accessed directly in `_evaluate_model_artifact` and `_forecast_model_artifact` via `add_config()` to persist the artifact timestamp. | @@ -156,7 +156,11 @@ manager.config = { "run_type": "calibration", "level": "pg_id", "algorithm": "LocfModel", - "targets": ["y1", "y2"], + "regression_targets": ["y1", "y2"], + # CORE_GENOME keys — audit_manifest raises before the catalog is reached without them + "steps": [*range(1, 37)], + "time_steps": 36, + "prediction_format": "prediction_frame", } # Training model = manager._train_model_artifact() # fits, pickles, returns model @@ -196,15 +200,25 @@ preds = mgr._evaluate_model_artifact(eval_type="temporal") ```python # Using an algorithm name not registered in the catalog -manager.config = {"run_type": "eval", "level": "pg_id", "algorithm": "SVR", "targets": ["y1"]} +manager.config = { + "run_type": "eval", "level": "pg_id", "algorithm": "SVR", + "regression_targets": ["y1"], "steps": [1], "time_steps": 36, + "prediction_format": "prediction_frame", +} manager._evaluate_model_artifact(eval_type="temporal") -# ValueError: "Model 'SVR' is not in the catalog. Available: ..." +# MissingHyperparameterError: "Unknown algorithm 'SVR'. Available: [...]" +# (the GATE rejects it first — the catalog's own ValueError is unreachable in a +# pipeline run, though still raised on the direct-API path) # Missing required config key -manager.config = {"run_type": "eval", "level": "pg_id", "algorithm": "AverageModel", "targets": ["y1"]} +manager.config = { + "run_type": "eval", "level": "pg_id", "algorithm": "AverageModel", + "regression_targets": ["y1"], "steps": [1], "time_steps": 36, + "prediction_format": "prediction_frame", +} # Missing "window_months" manager._evaluate_model_artifact(eval_type="temporal") -# ValueError: "Model 'AverageModel' requires config keys ['window_months'] but they are missing" +# MissingHyperparameterError: "Algorithm 'AverageModel' requires missing parameters: ['window_months']" # Indexing _forecast_model_artifact by position (it returns dict[str, PredictionFrame], keyed by target) forecast = manager._forecast_model_artifact() diff --git a/docs/CICs/BaselineModelCatalog.md b/docs/CICs/BaselineModelCatalog.md index fadb3ef..34ec65c 100644 --- a/docs/CICs/BaselineModelCatalog.md +++ b/docs/CICs/BaselineModelCatalog.md @@ -48,7 +48,7 @@ | Parameter | Type | Notes | |---|---|---| -| `config` | `dict` | Must contain `"targets"` (List[str]) for all models. Model-specific keys per `MODEL_GENOMES`. | +| `config` | `dict` | Must contain `"regression_targets"` (List[str]) for all models — read once in `__init__` as `self.targets`. Model-specific keys per `MODEL_GENOMES`, read lazily in `get_model()`. | | `partition_dict` | `dict` | Passed directly to each model constructor. Must contain `"test"` key. | | `loa` | `str` | Level-of-analysis string. Passed directly to each model constructor. | @@ -70,7 +70,7 @@ The catalog stores a reference to `config` (not a copy). Mutations to `config` a |---|---|---| | Unknown `model_name` | `ValueError` with message listing available names | `"Model '{model_name}' is not in the catalog. Available: ..."` | | Required config key missing | `ValueError` listing missing keys | `"Model '{model_name}' requires config keys {missing} but they are missing"` | -| `config` missing `"targets"` | `KeyError` (crash) | No explicit validation; `targets=self.config["targets"]` raises. | +| `config` missing `"regression_targets"` | `KeyError` at **construction**, not at `get_model()` | Declared in `CORE_GENOME` since #85, so a pipeline run fails earlier and louder at `audit_manifest`. A direct caller that skips the gate still gets a `KeyError` — now from `__init__`, where the key is read, rather than from inside a factory method. | | `partition_dict` missing `"test"` | Deferred to model | Not validated in catalog; surfaces when the model calls `partition_dict["test"]`. | --- @@ -92,18 +92,19 @@ from views_baseline.model.catalog import BaselineModelCatalog partition_dict = {"test": (493, 528)} # ZeroModel — no model-specific keys required -config = {"targets": ["y1", "y2"]} +config = {"regression_targets": ["y1", "y2"]} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") model = catalog.get_model("ZeroModel") # AverageModel — requires "window_months" -config = {"targets": ["y1", "y2"], "window_months": 6} +config = {"regression_targets": ["y1", "y2"], "window_months": 6} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") model = catalog.get_model("AverageModel") assert model.window_months == 6 # MixtureBaseline — all model-specific params required -config = {"targets": ["y1"], "window_months": 18, "lambda_mix": 0.05, "n_samples": 256} +config = {"regression_targets": ["y1"], "window_months": 18, "lambda_mix": 0.05, + "n_samples": 256, "seed": 42} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") model = catalog.get_model("MixtureBaseline") assert model.window_months == 18 @@ -124,22 +125,27 @@ catalog.get_model("BayesianModel") # ValueError: "Model 'BayesianModel' is not in the catalog. Available: ..." # Missing required key for AverageModel -config = {"targets": ["y1"]} # no "window_months" +config = {"regression_targets": ["y1"]} # no "window_months" catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") catalog.get_model("AverageModel") # ValueError: "Model 'AverageModel' requires config keys ['window_months'] but they are missing" -# Missing "targets" in config (not in MODEL_GENOMES — crashes rather than raises ValueError) +# Missing "regression_targets" — now fails at CONSTRUCTION, not inside a factory config = {"window_months": 6} +BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") +# KeyError: 'regression_targets' from __init__ +# (In a pipeline run this never gets that far: `regression_targets` is in CORE_GENOME, +# so audit_manifest raises MissingHyperparameterError naming the key first.) + +# Mutating config after construction still affects MODEL-SPECIFIC keys, which are read +# lazily in get_model() — but NO LONGER affects targets, which are copied in __init__. +config = {"regression_targets": ["y1"], "window_months": 6} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") -catalog.get_model("AverageModel") # KeyError: "targets" inside factory method - -# Mutating config after construction affects subsequent get_model calls -config = {"targets": ["y1"]} -catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pg_id") -config["targets"] = [] # mutation visible inside catalog -model = catalog.get_model("ZeroModel") -assert model.targets == [] # unexpected empty targets +config["regression_targets"] = [] # no longer visible to the catalog +config["window_months"] = 999 # still visible +model = catalog.get_model("AverageModel") +assert model.targets == ["y1"] # copied at construction (#85) +assert model.window_months == 999 # read at get_model() time ``` --- @@ -170,11 +176,11 @@ Files: `tests/test_catalog.py` and `tests/test_baseline.py` (the `list_models` c ## Evolution Notes - To add a new model, register it in both `MODEL_GENOMES` and `self.models`, and add a `_get_*` factory method. All model-specific parameters must be listed in `MODEL_GENOMES` — the catalog does not apply defaults. -- If `"targets"` should also be validated (currently not in genomes), it can be added as a universal required key checked outside the model-specific genome logic. +- ~~If `"targets"` should also be validated…~~ **Done (#85):** `regression_targets` and `level` are declared in `CORE_GENOME`, so both are audited by `audit_manifest` before the catalog is constructed. --- ## Known Deviations -- The `"targets"` key is universally required for all models but is not listed in any `MODEL_GENOMES` entry. A missing `"targets"` key surfaces as a `KeyError` inside the factory method rather than a descriptive `ValueError` from the genome check. -- The catalog stores a reference to `config`, not a copy. Post-construction mutations to the dict affect factory behaviour. +- ~~The `"targets"` key is universally required … surfaces as a `KeyError` inside the factory method~~ — **resolved 2026-09-09 (#85).** This deviation was recorded in April 2026 and was the shape of the August outage: pipeline-core retired the key, the catalog kept reading it, and the failure was the predicted opaque `KeyError`. Now `regression_targets` is in `CORE_GENOME` and is read once in `__init__`. +- The catalog stores a reference to `config`, not a copy, so post-construction mutations still affect the **model-specific** keys read lazily in `get_model()`. The target list is no longer affected: `self.targets = list(config["regression_targets"])` copies at construction (#85). This is a partial, not a full, fix — noted rather than extended, since no caller mutates a config mid-construction today. diff --git a/docs/CICs/ReproducibilityGate.md b/docs/CICs/ReproducibilityGate.md index e6b4eb8..91b4a7c 100644 --- a/docs/CICs/ReproducibilityGate.md +++ b/docs/CICs/ReproducibilityGate.md @@ -25,7 +25,7 @@ The class is importable by downstream packages (e.g. views-models) so they can v ## Responsibilities and Guarantees -**`Config.CORE_GENOME`**: Class attribute. List of config keys required by every baseline model: `["steps", "time_steps", "prediction_format"]`. +**`Config.CORE_GENOME`**: Class attribute. List of config keys required by every baseline model: `["steps", "time_steps", "prediction_format", "regression_targets", "level"]`. `regression_targets` and `level` were promoted in #85 — both are dereferenced unconditionally on every run (`level` by the manager one line after this audit returns, `regression_targets` by all seven catalog builders), so their absence must be a named contract violation rather than a `KeyError` deeper in. **`Config.ALGORITHM_GENOMES`**: Class attribute. Dict mapping each algorithm name to its list of additional required config keys. This dict is the single source of truth; `BaselineModelCatalog.MODEL_GENOMES` is an alias to it. @@ -85,9 +85,11 @@ from views_baseline.infrastructure.reproducibility_gate import ReproducibilityGa # Valid config — passes silently config = { "algorithm": "AverageModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, + "prediction_format": "prediction_frame", "window_months": 60, } ReproducibilityGate.Config.audit_manifest(config) # no error @@ -96,8 +98,13 @@ ReproducibilityGate.Config.audit_manifest(config) # no error CORE_PARAMS = set(ReproducibilityGate.Config.CORE_GENOME) ALGO_PARAMS = ReproducibilityGate.Config.ALGORITHM_GENOMES -hp = get_hp_config() # from config_hyperparameters.py -missing = CORE_PARAMS - set(hp.keys()) +# NOTE: validate the MERGED config, not config_hyperparameters.py alone. `level` and +# `prediction_format` live in config_meta.py in every shipped baseline, and +# `regression_targets` is meta-only in 11 of 29 — so checking `hp` alone reports every +# healthy model as broken. (Before #85 this snippet happened to work because CORE_GENOME +# held only hyperparameter-file keys; it no longer does.) +merged = {**get_hp_config(), **get_meta_config()} +missing = CORE_PARAMS - set(merged.keys()) assert not missing, f"Config missing core params: {missing}" ``` @@ -107,17 +114,26 @@ assert not missing, f"Config missing core params: {missing}" ```python # Missing core key — raises -config = {"algorithm": "ZeroModel", "targets": ["y1"], "time_steps": 36} +config = { + "algorithm": "ZeroModel", "regression_targets": ["y1"], "level": "pgm", + "time_steps": 36, "prediction_format": "prediction_frame", +} ReproducibilityGate.Config.audit_manifest(config) # MissingHyperparameterError: Missing core parameters: ['steps'] # None value — raises -config = {"algorithm": "ZeroModel", "targets": ["y1"], "steps": None, "time_steps": 36} +config = { + "algorithm": "ZeroModel", "regression_targets": ["y1"], "level": "pgm", + "steps": None, "time_steps": 36, "prediction_format": "prediction_frame", +} ReproducibilityGate.Config.audit_manifest(config) # MissingHyperparameterError: Mandatory parameters set to None: ['steps'] # Unknown algorithm — raises -config = {"algorithm": "SVR", "targets": ["y1"], "steps": [1], "time_steps": 36} +config = { + "algorithm": "SVR", "regression_targets": ["y1"], "level": "pgm", + "steps": [1], "time_steps": 36, "prediction_format": "prediction_frame", +} ReproducibilityGate.Config.audit_manifest(config) # MissingHyperparameterError: Unknown algorithm 'SVR'. Available: [...] ``` diff --git a/docs/contributor_protocols/hardened_protocol_template.md b/docs/contributor_protocols/hardened_protocol_template.md index cc990b7..5f0db3d 100644 --- a/docs/contributor_protocols/hardened_protocol_template.md +++ b/docs/contributor_protocols/hardened_protocol_template.md @@ -93,8 +93,7 @@ Rules: apply defaults — all values must be explicitly declared in `config_hyperparameters.py`. - Parameters (those with defaults) are **not** listed in `MODEL_GENOMES`. They are accessed via `config.get(key, default)` in the factory method. -- `targets` and `partition_dict` are universal inputs (all models require them) but are not - listed in `MODEL_GENOMES` — this is a known gap documented in ADR-009. +- `partition_dict` is a universal input (all models require it) but is not listed in `MODEL_GENOMES`. (`regression_targets` WAS in this position and is no longer: it is declared in `CORE_GENOME` as of #85, after the gap this sentence described became a five-week outage — see ADR-009.) When adding a new model, update `MODEL_GENOMES` in the same commit as the constructor. A `MODEL_GENOMES` entry that does not match the constructor is a silent validation gap. diff --git a/docs/guides/publishing-to-pypi.md b/docs/guides/publishing-to-pypi.md index ddbc98c..956a589 100644 --- a/docs/guides/publishing-to-pypi.md +++ b/docs/guides/publishing-to-pypi.md @@ -13,7 +13,7 @@ months later** — every command is copy-paste-able. | Thing | What | Why it matters | |---|---|---| -| **⚠️ Dependency not on PyPI yet** | `views-baseline` depends on `views-pipeline-core >=3.0.0`, which is **not published on PyPI** | Uploading the wheel **succeeds** and reserves the name/version, but `pip install views-baseline` will **fail to resolve** until pipeline-core 3.0.0 is on PyPI. `views-frames` (the other dep) *is* on PyPI. A clean-room install-back (§A) is therefore blocked until then. | +| **✅ Dependencies now resolve** | `views-pipeline-core` reached PyPI on **2026-08-03** (3.0.0; now 3.2.0) | Historical note: 1.0.0 and 1.0.1 were published while this dependency had no PyPI release, so `pip install views-baseline` could not resolve *any* version (register C-38). That is cleared. **1.0.2 is the first release for which a clean-room install-back (§A) actually works — do it, since it has never been exercised.** | | **Single package** | The `views-baseline` wheel ships one import package, `views_baseline` | `pip install views-baseline` → `import views_baseline`. | | **Versions are write-once** | Once `X.Y.Z` is on PyPI it can never be re-uploaded or truly deleted (only "yanked") | Always **bump the version first**. For repeated TestPyPI rehearsals use a throwaway like `1.0.1.dev1`. | | **uv + hatchling, NOT poetry** | Build backend is `hatchling.build`; tooling is `uv` | Use `uv build` / `uv publish`. | @@ -51,9 +51,11 @@ trusted-publisher config — see Prerequisites. The release workflow authenticates with **Trusted Publishing (OIDC)** — there is **no stored token**. A project owner enables it **once** on PyPI. -**Because `views-baseline` is not on PyPI yet, use a _pending_ publisher** (PyPI lets you -trust a publisher for a project that does not exist yet; the first OIDC publish then -creates the project): +**Already done — this is history, not a step.** The project now exists on PyPI (1.0.0 published +2026-07-31, 1.0.1 on 2026-08-02), so the trusted publisher is a normal one and needs no further +setup. It was originally registered as a **pending** publisher, which is what PyPI offers for a +project that does not exist yet; the first OIDC publish created the project. Kept for the next +person setting up a *new* package from this template: > PyPI → your account → **Publishing** → **Add a pending publisher (GitHub)**: > - **PyPI Project Name:** `views-baseline` diff --git a/pyproject.toml b/pyproject.toml index 983e741..bec4501 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,6 +47,19 @@ Homepage = "https://github.com/views-platform/views-baseline" Repository = "https://github.com/views-platform/views-baseline" Changelog = "https://github.com/views-platform/views-baseline/releases" +# uv resolves a UNIVERSAL lockfile across every platform its markers admit, including +# win32. views-pipeline-core -> viewser -> docker<6.0.0 -> pywin32{sys_platform=='win32'}==227, +# and pywin32 227 published no wheels above cp39, so the win32 split is unsatisfiable on +# Python 3.11-3.13 and `uv sync` fails on Linux CI for a platform CI never runs. +# +# Constraining resolution to Linux is uv's documented remedy for exactly this (it is the +# hint uv itself emits). The published wheel is unaffected: it stays `py3-none-any` with no +# platform markers of its own — this bounds the DEV/CI resolution, not the distribution. +# Revisit if a Windows or macOS contributor needs `uv sync`; the conda `views_pipeline` env +# is the sanctioned dev path today and it is Linux. +[tool.uv] +environments = ["sys_platform == 'linux'"] + [dependency-groups] dev = [ "pytest>=8.0", diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index bb07f7a..786835b 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -4,9 +4,9 @@ |-------------------|--------------------------------------| | Project | views-baseline | | Owner | Project maintainers | -| Last Updated | 2026-07-31 | -| Total Concerns | 38 | -| Open Concerns | 12 | +| Last Updated | 2026-09-09 | +| Total Concerns | 52 | +| Open Concerns | 26 | | Resolved Concerns | 25 | | Withdrawn | 1 (C-27 — Tweedie removed) | @@ -33,13 +33,13 @@ isolation. | Cluster | Root cause | Entries | Status | |---------|-----------|---------|--------| -| K1 — Frames boundary (DIP/SDP) | Model layer coupled to concrete platform data types; no baseline-owned seam | C-16, C-08, C-31, C-19, C-05, C-18 | Resolved via `to_prediction_frames` + `to_feature_frame` (ADR-019/020); only C-19 OCP half deferred (ADR-012) | +| K1 — Frames boundary (DIP/SDP) | Model layer coupled to concrete platform data types; no baseline-owned seam | C-16, C-08, C-31, C-19, C-05, C-18, C-39 | Seam resolved via `to_prediction_frames` + `to_feature_frame` (ADR-019/020); C-19 OCP half deferred (ADR-012); **C-39 new** — the manager-side format dispatch can silently bypass the frame path | | K2 — numpy-port byte-identity (epic #47) | pandas→numpy rewrite risking silent forecast drift + weak guard tests | C-32, C-33, C-34, C-35, C-36, C-37, C-29 | Resolved & merged (PR #60); only C-30 (numpy pin) deferred | | K3 — Seed / reproducibility | Seed not wired; determinism-only tests | C-10, C-24, C-25, C-29 | Resolved & merged (ADR-021, #31) | -| K4 — PredictionFrame-as-sampled ecosystem | Platform treats PF as inherently distributional; point models don't fit | C-11, C-13, C-14, C-20 | Open, cross-repo (pipeline-core / views-models) | -| K5 — Input-boundary validation | Boundary doesn't validate index order / partition_dict / targets | C-05, C-06, C-07 | Partly subsumed by K1; C-06/C-07 residual | -| K6 — Governance-doc drift | Docs lag code changes | C-21, C-28 | C-28 resolved; C-21 residual sites | -| K7 — Distribution / packaging | Cross-repo dependency on unpublished pipeline-core | C-38, C-17 | C-17 superseded; C-38 live, blocked on pipeline-core #319 | +| K4 — PredictionFrame-as-sampled ecosystem | Platform treats PF as inherently distributional; point models don't fit | C-11, C-13, C-14, C-20, C-44 | Open, cross-repo (pipeline-core / views-models); C-44 adds the private-API surface all of these are asserted across | +| K5 — Input-boundary validation | Boundary doesn't validate index order / partition_dict / targets | C-05, C-06, C-07, C-42, C-48 | Data-side closed (C-06/C-07). Config-side **largely closed by #85**: `regression_targets` + `level` declared, and empty values now rejected (C-42). Residual: `run_type` read past the gate (C-48) | +| K6 — Governance-doc drift | Docs lag code changes | C-21, C-28, C-40 | C-21/C-28 resolved; **third recurrence open** — eight ADRs cite files deleted by the epic #47 reorg (C-40). No check added when the first two closed | +| K7 — Distribution / packaging | Dependency declaration and installability | C-38, C-17, C-41, C-47 | C-17 superseded; **C-38 discharge condition met** (pipeline-core 3.0.0 on PyPI 2026-08-03; CI resolves); C-41 re-scoped — scipy arrives transitively, the defect is the undeclared *direct* import; C-47 new — 3.12/3.13 unusable | --- @@ -195,7 +195,7 @@ See also C-13 (related root cause: the ecosystem treating PredictionFrame as inh | Tier | 3 | | Source | expert-review (2026-06-24) | | Trigger | When a contributor applies the #21 migration only to its two *listed* sites (`helpers.py`, `baseline.py:285`) — `MixtureBaseline.predict()` at `baseline.py:402` retains the old `identifiers=` constructor and ships half-migrated; and when adding a sixth model, three parallel registries (catalog dispatch dict, `ALGORITHM_GENOMES`, `_get_*` method) must be edited in lockstep or the model is silently unavailable/unvalidated | -| Location | `views_baseline/model/baseline.py:285,402` (two of three construction sites), `views_baseline/model/helpers.py:136` (the third); `views_baseline/model/catalog.py:24-30` + `views_baseline/infrastructure/reproducibility_gate.py:27` + `catalog.py:53-88` (three parallel registries) | +| Location | **Post-reorg (2026-09-09):** `views_baseline/model/catalog.py:24-32` (dispatch dict) + `catalog.py:55-120` (seven `_get_*` factories) + `views_baseline/infrastructure/reproducibility_gate.py:27-41` (`ALGORITHM_GENOMES`) + `views_baseline/model/models/{point,distributional}/__init__.py` (re-export lists) + the `pytest.mark.parametrize` lists in `tests/test_protocol.py`, `tests/test_catalog.py`. **Pre-reorg (historical, files now deleted):** `model/baseline.py:285,402`, `model/helpers.py:136` | `PredictionFrame` is constructed at three independent sites and #21's location list names only two of them — an omission surface that ships a working point/Conflictology path while `MixtureBaseline` (`baseline.py:402`) breaks. Separately, adding a model is an Open/Closed violation: it requires coordinated edits to the catalog dispatch dict, the genome table, and a `_get_*` factory method (ADR-012 codifies the ritual). Both are the same structural issue — construction and registration logic spread across files instead of localized behind one seam. No correctness impact today beyond the half-migration risk; the cost is maintainability and a recurring edit surface for every platform change and every new model. Mitigation: collapse all frame construction into one boundary adapter (point and sample), and replace the three registries with a single registration table keyed by model name. @@ -205,6 +205,8 @@ See also C-08 (the duplicated lazy import the adapter subsumes), C-16 (the migra > **Re-confirmed (2026-07-20, falsify probe P2):** the PR-1 reorg did not change the OCP posture — `catalog.py` still hardcodes a 7-entry dispatch dict + seven `_get_*` methods, so adding a model remains a modification of existing files plus an `ALGORITHM_GENOMES` edit (not a pure extension). Guarded by `tests/test_falsification_reorg_principles.py::test_ocp_catalog_does_not_hardcode_model_registry` (strict xfail — ratchets when a single registration table replaces the three registries). Consolidation stays deferred per ADR-012 (catalog < ~10 models). +> **Re-confirmed (2026-09-09, repo-assimilation):** the registry count has grown from three sites to **five**. The epic #47 reorg added the per-package re-export lists (`models/point/__init__.py`, `models/distributional/__init__.py`) as a fourth coordinated edit, and the model-parametrized test lists are a fifth; the construction half remains resolved (one `to_prediction_frames` seam). Location field refreshed to post-reorg paths — the pre-reorg paths recorded above are retained as history and are also cited as *current* by eight ADRs (see C-40). Consolidation still deferred per ADR-012. + --- ### C-20: `views_frames.PredictionFrame` silently accepts zero-sample `(N, 0)` arrays @@ -292,6 +294,292 @@ views-baseline 1.0.0 was uploaded to PyPI via Trusted Publishing, reserving the See also C-17 (the prior pin-hygiene entry — **inverted** here: 3.0.0 is now intentionally *required* rather than dangerously *admitted*), C-16 (the code break the `>=3.0.0` floor exists to require the fix for), and C-31/C-32 (the FeatureFrame-native work that made the frame path pandas-free, orthogonal to this distribution gap). +> **DISCHARGE CONDITION MET (2026-09-09).** views-pipeline-core reached PyPI on **2026-08-03** +> (3.0.0), and is now at 3.2.0 — 3.0.1 (08-11), 3.1.0 (08-13), 3.1.1 (08-14), 3.1.2 (08-26), +> 3.2.0 (09-08). `views-baseline`'s `>=3.0.0,<4.0.0` floor therefore resolves, which is precisely +> why teammates *can* install 1.0.1 and hit the #84 outage. Both halves of this entry are cleared by +> events: the install block is gone, and the red CI it caused is being verified in #90. Moves to +> **Resolved** when #92 publishes 1.0.2 and a clean-room `pip install` is demonstrated. +> +> **Second-order consequence recorded (2026-09-09, repo-assimilation).** The red `check` matrix is not merely a cosmetic CI failure — it means **no merge to `development` or `main` is gated on the test suite at all**. Of the three jobs in `run_tests.yml`, only `lint` (`uvx ruff check .`, project-independent) and `build` (`uv build`) produce a meaningful signal; the 248-test suite executes solely in a maintainer's local editable `views_pipeline` conda env, at their discretion and on their machine. Every correctness guard the repo has invested in — the golden byte-identity tests, the pandas-free-path subprocess tripwire, the strict-xfail ratchets — is therefore advisory rather than enforced, and this holds for any contributor who does not have the editable env. This is the enabling condition beneath C-39 and C-44 (both would be caught by an integration test that cannot currently run in CI). It discharges at the same moment as the entry above: when pipeline-core 3.0.0 reaches PyPI, `uv sync` resolves and the suite becomes a real gate. Until then, branch protection should not be read as test protection. + +--- + +### C-39: Frame-declared run silently degrades to the pandas path via the `_data_format` `getattr` default + +| Field | Value | +|-------|-------| +| ID | C-39 | +| Tier | 2 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When views-pipeline-core renames, relocates, or stops setting the private `_data_format` attribute on `ForecastingModelManager` (or changes its assignment to run after `_load_source` is first called) — verify that a `data_format: feature_frame` config still takes the `load_frame_cache` branch rather than silently falling through to `read_dataframe` | +| Location | `views_baseline/manager/baseline_manager.py:70` (the `getattr(self, "_data_format", DATA_FORMAT_DATAFRAME)` dispatch), `:53-72` (the docstring that forbids exactly this fallback) | + +`_load_source` dispatches the frame-native seam on `getattr(self, "_data_format", DATA_FORMAT_DATAFRAME)`. Its own docstring states the design intent explicitly — *"There is deliberately NO silent `getattr(..., None)` fallback from the frame path back to pandas: a frames-declared config that has lost its cache must fail loud, never silently degrade to a pandas run (pipeline-core register C-214)"* — but the dispatch expression itself is that fallback, entered through a different door. The guarded case is a *missing frame cache* (raised loudly by `_get_cached_frame_path()`); the unguarded case is a *missing or renamed attribute*, where the default silently selects pandas. Because both branches return a valid model input and every downstream model has accepted either shape since ADR-019, the run completes and emits plausible `PredictionFrame`s with no error, no warning, and no log line naming the format actually used — the declared `data_format` is simply not honoured. Likelihood is real rather than theoretical: `_data_format` is an underscore-private member of an external, actively-refactored package (see C-44), and no test would catch the rename because every manager test fakes the attribute (`tests/conftest.py:99-122` constructs via `__new__`). + +**Tier rationale (impact vs. likelihood):** the outcome is a silent, unsignalled substitution of the data path — the pandas branch is byte-identical by design today, so the immediate numeric impact is nil, which keeps this off Tier 1. But it defeats a declared contract without any signal and its trigger is a routine upstream refactor, which is precisely Tier 2 structural fragility. Mitigation would be an explicit presence check plus a log line stating the resolved format. + +See also C-44 (the private-API coupling that makes the trigger likely), C-09 (the sibling "enforced by convention only" topology gap). Part of causal cluster **K1 — Frames boundary**. + +--- + +### C-40: ADR source-path drift after the epic #47 reorg — eight ADRs cite deleted files, including the model-addition checklist + +| Field | Value | +|-------|-------| +| ID | C-40 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When a contributor follows ADR-012 to add a new baseline model, or runs ADR-013's stated compliance command `grep -n "from views_pipeline_core" views_baseline/model/baseline.py` to verify the coupling contract — both target files deleted by the reorg, so the checklist misdirects and the verification cannot execute | +| Location | `docs/ADRs/012_model_addition_protocol.md:25,29,164`; `docs/ADRs/013_pipeline_core_coupling.md:27,30,35,38,41,49,143,154,171`; `docs/ADRs/011_rng_determinism_contract.md:138,148,181`; `docs/ADRs/003_authority_of_declarations_over_inference.md:155,157,162,197`; `docs/ADRs/020_views_frames_adapter_and_level_contract.md:28,100`; `docs/ADRs/004_rules_for_evaluation_and_stability.md:175`; `docs/ADRs/018_predictionframe_use_in_views_baseline.md:29`; `docs/ADRs/021_zero_magic_and_explicit_defaults.md:89`; `docs/CICs/ConflictologyModel.md`, `docs/CICs/MixtureBaseline.md`; the missing check in `docs/validate_docs.sh` | + +The epic #47 reorg split `views_baseline/model/baseline.py` into `model/models/point/*.py` + `model/models/distributional/*.py` and `model/helpers.py` into `model/grid.py` + `model/frames/{input,output,pooling}.py`, but the ADR corpus was not swept. Eight ADRs and two CICs still give those paths as *current-state prescriptions*, not as history. Two sites are load-bearing rather than cosmetic: **ADR-012** is the authoritative model-addition checklist and its Step 1 reads `**File:** views_baseline/model/baseline.py`, also directing distributional models to "construct `PredictionFrame` objects directly" — the exact practice ADR-020's single-seam decision superseded; and **ADR-013** publishes a grep command against `model/baseline.py` as its compliance verification, so the coupling contract now has no working check. The drift is structurally invisible to the repo's own tooling: `docs/validate_docs.sh` validates header fields, template status, and cross-document links, but never that a **source path cited by a doc exists on disk**. Against a governance ratio of 3.3 lines of prose per line of source, a contributor's most likely first action is to trust the ADR over the tree. + +See also C-21 and C-28 (the two prior, now-resolved drift instances — this is the third recurrence, and the first to survive because no check was added when they were closed). Part of causal cluster **K6 — Governance-doc drift**. + +--- + +### C-41: `scipy` is an undeclared runtime dependency of the shipped `evaluation` module + +| Field | Value | +|-------|-------| +| ID | C-41 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When a reviewer or downstream user installs views-baseline from a clean index (once C-38 clears) and imports `views_baseline.evaluation.closeness` to reproduce the ADR-022 equivalence result — the import raises `ModuleNotFoundError: scipy` because no declared dependency supplies it | +| Location | `views_baseline/evaluation/closeness.py:18` (`from scipy.stats import energy_distance, wasserstein_distance`, module level); `pyproject.toml:31-43` (declares only `numpy`, `views-frames`, `views-pipeline-core`); `views_baseline/model/distributions/__init__.py:11` (docstring claims "numpy + scipy only" for a numpy-only package) | + +`evaluation/closeness.py` imports scipy at module level and is packaged into the wheel (`[tool.hatch.build.targets.wheel] packages = ["views_baseline"]` ships all of `views_baseline/`), yet scipy appears in no dependency list. Neither declared dependency supplies it transitively: `views-pipeline-core` declares no scipy requirement, and `views-frames` declares `scipy>=1.11,<2` only under its **`docs` extra**, which a normal install does not pull. The module is not incidental — it is the closeness harness that operationalises ADR-022's "indistinguishable from conflictology" verdict (C2ST / Wasserstein / energy distance) and backs `reports/closeness_experiment/FINDINGS.md`, so the first person to independently re-run the epic #33 evidence from a published wheel is the one who hits it. The failure is currently masked everywhere it would be noticed: the `views_pipeline` conda env has scipy installed, and CI never reaches the test step (C-38). A secondary inaccuracy compounds the confusion: `model/distributions/__init__.py` advertises a scipy dependency for a package whose families are implemented purely in numpy. + +See also C-17 (the same failure mode — an undeclared dependency working locally and only surfacing off the dev machine — previously registered for `views-frames` and resolved by declaring it). Part of causal cluster **K7 — Distribution / packaging**. + +> **TRIGGER CORRECTED (2026-09-09, #94 review).** The entry claimed the import "raises +> `ModuleNotFoundError: scipy`" on a clean install because "views-pipeline-core declares no scipy +> requirement". Directly true, and misleading: `views-baseline -> views-pipeline-core -> +> views-evaluation` requires `scipy>=1.11,<2.0` (and `views-transformation-library`, +> `scikit-learn` and `pyod` do too, all reached the same way), so **scipy is installed on any real +> install of this package** — verified at 1.15.1 in the working env, and `tests/test_closeness.py` +> passes on it. The stated failure cannot fire on the install path the trigger names, and the +> "masked because CI never reaches the test step" clause is retired by #90. +> +> The residual defect is real but different, and smaller: `evaluation/closeness.py` imports scipy +> **directly** while declaring nothing, so the package depends on a transitive pin it does not +> control. If views-evaluation drops scipy, this breaks with no signal from our own metadata. That +> is dependency hygiene (PEP 508: declare what you import), not a live install failure — the tier +> stays 3 but the trigger becomes "when views-evaluation or pipeline-core stops requiring scipy", +> not "when anyone installs". + +--- + +### C-42: The reproducibility gate does not audit `level` or `targets`, the two config keys every model path dereferences + +| Field | Value | +|-------|-------| +| ID | C-42 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When a contributor adds a new baseline model config in views-models (or trims an existing one) and omits `level` or `targets` — verify the failure is a `MissingHyperparameterError` naming the key, not a bare `KeyError` raised past a gate that has already declared the config valid | +| Location | `views_baseline/infrastructure/reproducibility_gate.py:24` (`CORE_GENOME = ["steps", "time_steps", "prediction_format"]`), `:27-41` (`ALGORITHM_GENOMES` — `targets` appears in no genome list); dereferenced unguarded at `views_baseline/manager/baseline_manager.py:79` (`self.config["level"]`) and `views_baseline/model/catalog.py:57,62,67-70,75-81,86-92,98-106,113-120` (`self.config["targets"]` in all seven factories) | + +`ReproducibilityGate.Config.audit_manifest` is the declared boundary contract for configuration (ADR-009/ADR-021): it exists so a malformed config fails at the boundary with `REPRODUCIBILITY CONTRACT VIOLATED` naming the missing key. Two keys escape it entirely. `level` is read by `_setup_model_and_data` immediately after the audit passes; `targets` is read by every one of the seven catalog factory methods and is listed in **no** genome, so `get_model`'s own `missing = [k for k in self.MODEL_GENOMES[model_name] if k not in self.config]` check cannot catch it either. A config missing either key therefore clears both validation layers and dies on a bare `KeyError` inside the object graph — the precise failure mode the gate was built to eliminate, and the one an ADR-009 reader would assume is impossible. The gate is also importable by views-models for *static* config validation, so the omission propagates: a downstream repo checking its configs against `CORE_GENOME` gets a false green. + +See also C-07 (the sibling gap on the *data* side — `targets` columns in the input frame — resolved by the `to_feature_frame` boundary; this is the unclosed *config* side of the same pair). Part of causal cluster **K5 — Input-boundary validation**. + +> **TIER CORRECTED + FIX IMPLEMENTED (2026-09-09, epic #85).** This entry was registered at +> **Tier 3** as a diagnostics concern — "a missing key gives a bare `KeyError` instead of a named +> contract violation" — with a *hypothetical future* trigger ("when a contributor omits `level` or +> `targets`"). Both judgements were wrong, and the register is more useful for saying so than for +> hiding it: +> +> - The trigger had **already fired**, five weeks earlier. views-pipeline-core retired the +> synthesised `targets` key on 2026-08-02 (#380); `catalog.py` kept reading it; **all 29 baseline +> models had been unrunnable since**. This was a live production outage, not a latent quality issue +> — Tier 1 by the register's own definition once the read side is counted. +> - The reason I mis-read it is itself the finding: I checked whether the key was *validated* and +> assumed it was *present*, because `tests/conftest.py` supplied it. I had described that exact +> trap in **C-44** one entry earlier and then walked into it. A fixture is not evidence about +> production. +> +> **Fix (working tree, epic #85):** the catalog reads `regression_targets`, derived once in +> `__init__`; `regression_targets` and `level` are declared in `CORE_GENOME` (#87); a conformance +> test builds all seven models from real merged views-models configs (#88). Stays **Open** until +> merged and released. Superseded in substance by #85; the ID is retained with its error visible. + +--- + +### C-43: Point and distributional models derive their prediction grid from different sources and emit different row orders + +| Field | Value | +|-------|-------| +| ID | C-43 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When a caller passes a different frame to `predict()` than to `fit()` — e.g. a future forecast path that fits on the training partition and predicts on a trimmed or extended panel, or a sweep that reuses a fitted model across partitions — verify that distributional models still cover the intended entity set, since they ignore the predict frame's entities entirely | +| Location | `views_baseline/model/models/point/locf.py:63-66` and `average.py:66-69`, `zero.py:52-54` (grid from the **predict** frame via `entities_at`) vs. `views_baseline/model/models/distributional/conflictology.py:65-71`, `mixture.py:97-104`, `parametric.py:87-95`, `parametric_hurdle.py:108-115` (grid from **fit-time** `self.entity_ids`); ordering divergence at `views_baseline/model/grid.py:78-91` (first-appearance) vs. `views_baseline/model/frames/pooling.py:109` (`np.unique`, ascending) | + +The two model families answer "which entities am I predicting for?" from different places. Point models read the predict frame, select entities present at `train_end` via `entities_at`, and filter against fitted state. Distributional models call `to_level(df)` — deliberately the *lightest* input boundary, taking only the spatial level — and then build the grid from `self.entity_ids` captured during `fit`, so the predict frame's contents have **no influence on the output grid at all**. Today `_setup_model_and_data` hands the identical `df_source` to both calls, so membership agrees and nothing is wrong; the risk is that this equality is an accident of one call site rather than an enforced invariant, and nothing in the code or tests asserts it. A related consequence is already live: the two families order rows differently for the same input (first-appearance vs. ascending entity id), so any downstream consumer that aligns positionally rather than through the `SpatioTemporalIndex` would mis-join point against distributional frames. A third, quieter effect: `sample_prediction_grid`'s `filter_entities(entity_ids, fitted_state, model_name)` call is a structural no-op for all four distributional models, since both arguments derive from the same `window_pool` return — the entity-drop warning that exists on the point path has no counterpart on the sample path. + +See also C-05 (the broader "index structure assumed, not validated" family, now mitigated at the boundary) and C-19 (the OCP registry — a new model must independently rediscover which convention to follow). + +--- + +### C-44: Four private views-pipeline-core members are load-bearing, and every test defines them by mock + +| Field | Value | +|-------|-------| +| ID | C-44 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When views-pipeline-core refactors any underscore-private member of `ForecastingModelManager` — specifically `_data_format`, `_get_cached_frame_path`, `_get_cached_data_path`, or `_resolve_evaluation_sequence_number` — verify views-baseline still runs, because the 449-line manager suite stubs all four and will stay green through the break | +| Location | `views_baseline/manager/baseline_manager.py:70-72` (`_data_format`, `_get_cached_frame_path()`, `_get_cached_data_path()`), `:97` (`_resolve_evaluation_sequence_number`), `:109-141` (`_model_path.get_latest_model_artifact_path`, `_config_manager.add_config`); mocked at `tests/conftest.py:90-122` (`__new__` bypass + `SimpleNamespace`) | + +The manager is the single sanctioned coupling point to views-pipeline-core (ADR-013), but the surface it couples to is **private API**: four underscore-prefixed members of the base `ForecastingModelManager`, none of which carry a stability guarantee. This is a normal cost of the template-method integration and is not itself a defect. What makes it a tracked risk is that the coupling is verified against nothing: `make_manager` constructs the manager through `__new__`, bypassing `__init__` entirely, and attaches hand-built `SimpleNamespace` stand-ins for `_model_path`, `_data_loader`, and `_config_manager` plus a fake `_resolve_evaluation_sequence_number`. The mock, not the dependency, therefore defines the contract — a pipeline-core rename would leave all 248 tests green while production breaks. There is no integration test that instantiates the real base class. Compounding it, CI cannot install pipeline-core at all today (C-38), so even an integration test would not run there. + +See also C-39 (the specific silent-degradation instance this coupling enables), C-09 (topology rules enforced by convention only), C-13/C-14 (the other cross-repo contracts asserted only from this side). Part of causal cluster **K4 — PredictionFrame-as-sampled ecosystem**. + +> **Instance confirmed (2026-09-09, epic #85).** This entry predicted that "the mock, not the +> dependency, defines the contract." That is what happened, on a different surface than the four +> private members named above: `tests/conftest.py` manufactured the `targets` config key that +> views-pipeline-core had deleted, so **248 tests stayed green for five weeks while every production +> model raised `KeyError`**. The suite could not fail, because the fixture had drifted out of contact +> with the dependency and nothing compared them. +> +> Partially mitigated by #88, which builds every model from a checked-in copy of a real merged +> views-models config and asserts the fixtures carry no key pipeline-core would reject. The residual +> risk is unchanged and worth restating: a *copy* can still drift. It catches a key rename — the +> failure that happened — not a value change, and it does not exercise the four private +> pipeline-core members, which remain verified only against `SimpleNamespace` stand-ins. + +--- + +### C-45: No public API surface — a published package whose every consumer imports deep private paths + +| Field | Value | +|-------|-------| +| ID | C-45 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When the next internal reorganisation moves or renames a module under `views_baseline/model/` or `views_baseline/manager/` — verify which downstream import statements break, since there is no top-level re-export to absorb the move and the package now carries a public version number | +| Location | `views_baseline/__init__.py` (empty), plus 6 further empty package inits: `model/__init__.py`, `manager/__init__.py`, `infrastructure/__init__.py`, `evaluation/__init__.py`, `model/frames/__init__.py`, `model/models/__init__.py`; consumers import `views_baseline.manager.baseline_manager` and `views_baseline.model.models.distributional` directly | + +The distribution root exports nothing — no names, no `__all__`, no docstring — so there is no stable façade between the internal module layout and the outside world. Downstream code (views-models, and every test in this repo) must reach through the full internal path: `from views_baseline.manager.baseline_manager import BaselineForecastingModelManager`, `from views_baseline.model.models.distributional import ConflictologyModel`. Every internal file move is therefore a breaking change for consumers, with nothing available to soften it. This is no longer hypothetical on two counts: epic #47 already performed exactly such a reorganisation (`model/baseline.py` → `model/models/**`), and the package now ships to PyPI under semantic versioning, where consumers reasonably expect a 1.x minor release not to move their import paths. The three inits that *do* re-export (`models/point`, `models/distributional`, `distributions`) show the pattern is understood — it simply stops one level below the root. + +See also C-40 (the doc corpus that still cites the pre-reorg paths — the same move, unabsorbed on the documentation side). + +--- + +### C-46: `MixtureBaseline`'s global pool is serialised into an artifact that exists only to carry a timestamp + +| Field | Value | +|-------|-------| +| ID | C-46 | +| Tier | 4 | +| Source | repo-assimilation (2026-09-09) | +| Trigger | When `MixtureBaseline` is first run at `pgm` level (~10k grid cells × the full training panel) rather than the `cm` level and toy fixtures exercised so far — check the size of the written `.pkl` against the artifact directory's budget | +| Location | `views_baseline/model/models/distributional/mixture.py:64-72` (`self.global_pool[t] = v[v > 0].astype(np.float64)` — all positive values across the whole training panel, per target), pickled at `views_baseline/manager/baseline_manager.py:44-51` | + +`MixtureBaseline.fit` retains, per target, every positive value in the entire training panel as a float64 array — deliberately, since the global pool is what avoids the zero-probability trap, and deliberately float64 to keep the seeded `rng.choice` stream byte-identical (ADR-011). `_train_model_artifact` then pickles the whole fitted object. The artifact, however, is never unpickled: `_evaluate_model_artifact` and `_forecast_model_artifact` both re-fit from scratch and read only `path_artifact.stem[-15:]` — the file exists solely so a downstream ensemble can resolve a timestamp from its filename. The result is that the largest fitted state in the package is written to disk on every training run for a filename. No correctness impact; the cost is disk and serialisation time, and it scales with the panel rather than with the window. + +See also C-32 (the float64 retention is the byte-identity requirement that makes the pool expensive), C-22 (the same artifact's missing provenance metadata). + +--- + +### C-47: `views-baseline` declares support for Python 3.12/3.13 but cannot be installed on either + +| Field | Value | +|-------|-------| +| ID | C-47 | +| Tier | 3 | +| Source | repo-assimilation (2026-09-09, found by the #90 CI restoration) | +| Trigger | When anyone runs `pip install views-baseline` on Python 3.12 or 3.13 — trusting `requires-python = ">=3.11,<3.14"` or the `Programming Language :: Python :: 3.12/3.13` classifiers — the install fails building `pandas` from source. Also fires whenever someone restores the full CI matrix, which will go red again for this reason and not an obvious one. | +| Location | `pyproject.toml:14` (`requires-python`), `:22-23` (the 3.12/3.13 classifiers); `.github/workflows/run_tests.yml` (matrix narrowed to 3.11 with the explanation); external root cause `viewser 6.6.4` (`pandas<2.0.0,>=1.4.0`) reached via `views-pipeline-core>=3.0.0` | + +`views-pipeline-core` depends on `viewser>=6.6.4`, which pins `pandas<2.0.0,>=1.4.0`. The last pandas 1.x release, 1.5.3, publishes wheels for **cp38-cp311 only**. On Python 3.12 or 3.13 the resolver therefore has to build pandas 1.5.3 from source, which fails immediately — `ModuleNotFoundError: No module named 'pkg_resources'` under modern setuptools build isolation, and would very likely fail on Cython/numpy incompatibility even if that were patched. The package's declared support for 3.12 and 3.13 is consequently **false, and has been false since the `>=3.0.0` pipeline-core floor was adopted** — it is not a regression introduced by the #85 work, which merely made it visible by getting CI far enough to resolve dependencies at all. + +The metadata is deliberately **not** narrowed here. What a distribution claims to support is a release decision with downstream visibility, the constraint belongs to a dependency two hops upstream, and narrowing then re-widening `requires-python` across releases is worse than stating the gap once. The CI matrix is narrowed to 3.11 instead, so the pipeline is honestly green rather than permanently two-thirds red — the permanently-red condition being exactly what let C-42/#84 survive five weeks unnoticed. + +**Discharge:** when `viewser` drops the `pandas<2` pin (pandas 2.x supports 3.12/3.13), restore the full matrix and confirm. If viewser does not move, narrow `requires-python` and the classifiers at the next release instead. See also C-38 (the prior packaging-claim entry, discharged), C-41 (undeclared `scipy` — the other way this package's dependency metadata is wrong). Part of causal cluster **K7 — Distribution / packaging**. + +--- + +### C-48: `run_type` is dereferenced one line after the gate passes, and is not audited + +| Field | Value | +|-------|-------| +| ID | C-48 | +| Tier | 3 | +| Source | expert-review (2026-09-09, #94 review finding 3) | +| Trigger | When a caller reaches `_train_model_artifact` with a config that has no `run_type` — a direct-API user, a downstream static validation built on the contract this gate advertises, or a pipeline-core change to when `run_type` is injected — the audit reports success and the next line raises a bare `KeyError: 'run_type'`, the exact #84 shape | +| Location | `views_baseline/manager/baseline_manager.py:44-46` (`_setup_model_and_data()` runs the audit, then `self.config["run_type"]`); absent from `views_baseline/infrastructure/reproducibility_gate.py` `CORE_GENOME`; `views-pipeline-core` `configuration.py:464` declares `_SAFETY_CRITICAL_KEYS = frozenset({"level", "run_type", "prediction_format"})` | + +Epic #85 closed this gap for `level` and `regression_targets` and left it open one key over. pipeline-core names an authoritative trio of safety-critical keys; `CORE_GENOME` now covers two of the three. + +It is **deliberately not** in `CORE_GENOME`, and the reason is the one C-24 taught: `run_type` is injected at runtime (it is a CLI argument, not a config-file key), so requiring it would break the *static* downstream validation this class exists to enable — the very contract its docstring advertises and C-42's sibling entry is about. Promoting it demands the same coordinated verification C-24 required for `seed`, and #85 did not do that work. + +The honest interim is a documented asymmetry rather than a silent one: `tests/test_reproducibility_gate.py`'s `_KEYS_DEREFERENCED_ON_EVERY_RUN` now records `run_type`'s exclusion and why, so the completeness guard no longer certifies an incomplete list as complete. **Discharge:** verify `run_type` is present in the merged config at audit time for all run types, then promote it — or record that it cannot be, and move the read before the audit. See also C-24 (the sequencing precedent), C-42, C-44. + +--- + +### C-49: CI runs and passes but is not a required status check — green is advisory, not blocking + +| Field | Value | +|-------|-------| +| ID | C-49 | +| Tier | 3 | +| Source | expert-review (2026-09-09, #94 review finding 6) | +| Trigger | When a contributor merges a PR whose `check (3.11)` job is red — GitHub will not stop them, because neither branch declares required status checks. Also fires if someone enables required checks using a job name from a pre-2026-09-09 run (`check (3.12)`/`check (3.13)`), which no longer exist and would block every PR forever | +| Location | GitHub branch protection for `main` and `development` (both return `required_status_checks: null`); `.github/workflows/run_tests.yml` | + +Epic #85's premise is that "nothing was gating merges" is a root cause of the five-week #84 outage. #90 fixed the *running* half — the suite now resolves and passes in CI for the first time since 2026-08-02 — but not the *gating* half: a red run still does not block a merge on either protected branch. `development` requires no review either, so a broken PR can land unimpeded. + +This is a repository-settings change, not a code change, which is why it is registered rather than done: it is the maintainer's call, and it should be made now that the job is reliably green rather than while it was red. **Discharge:** add `check (3.11)`, `lint` and `build` as required status checks on `development` and `main`. Note the job-name hazard above when doing so. See also C-38/C-47 (why the job could not run), C-44. + +--- + +### C-50: the catalog reimplements pipeline-core's target vocabulary instead of consuming `combined_targets()` + +| Field | Value | +|-------|-------| +| ID | C-50 | +| Tier | 3 | +| Source | expert-review (2026-09-09, #94 review finding 8) | +| Trigger | When views-pipeline-core retires or renames `regression_targets` the way it retired `targets` in #380 — this package breaks identically, and **every conformance test stays green**, because `tests/test_config_conformance.py` hardcodes the same literal the catalog does | +| Location | `views_baseline/model/catalog.py` (`self.config["regression_targets"]`); `tests/test_config_conformance.py` (`MERGED_CONFIGS` literals); the unused alternative is `views_pipeline_core.managers.configuration.configuration.combined_targets`, used at 13+ read sites inside pipeline-core | + +`combined_targets(config)` is the canonical accessor pipeline-core created *in the same change* that caused #84, on a package already declared as a hard dependency. views-baseline is now the only reader in the platform reimplementing the vocabulary by hand. + +Calling it directly from `model/catalog.py` is blocked by ADR-013 (no module-level pipeline-core imports under `model/`), and routing it through the manager was rejected in #85 on the grounds that it would leave one of seven config keys arriving by a different mechanism than the other six — a split convention. Both remain true, so this is a **recorded consequence of a deliberate decision, not an oversight**. + +What the decision costs, stated plainly so a future reader can re-weigh it: the guard added in #88 cannot detect the failure it was built for. It pins the fixtures to the same literal as the code, so a rename moves both together and the tests pass while every model dies. Only the views-models runtime smoke test would catch it. **Discharge:** either adopt `combined_targets` at the manager seam (accepting the split convention) or add a test that asserts our literal still matches pipeline-core's derivation. See also C-19 (the OCP registry this is entangled with), C-42, D-04. + +--- + +### C-51: the lint gate resolves an unpinned `ruff`, so an upstream release can red every open PR + +| Field | Value | +|-------|-------| +| ID | C-51 | +| Tier | 4 | +| Source | expert-review (2026-09-09, #94 review addendum) | +| Trigger | When Astral stabilises or changes a rule in the selected `E`/`F`/`W`/`I` sets — every open PR goes red on code that passes locally under the dev-group ruff, with no repo change to bisect | +| Location | `.github/workflows/run_tests.yml` (`uvx ruff check .`, no version); `pyproject.toml` `[dependency-groups] dev` (`ruff>=0.8`, a floor with no ceiling) | + +The lint job deliberately runs project-independently so it stays green when the project cannot resolve — which was the right call while `check` was red, and is why it was the only signal for five weeks. The cost is that it pins nothing: `uvx ruff` takes whatever is newest at job time, which is also a different version than the dev group installs locally. This is the "gate red for a reason nobody re-diagnosed" class the workflow header now spends ten lines apologising for, one layer up. Low tier because the failure is loud, immediate, and fixed by a one-line pin. **Discharge:** pin `uvx ruff@` in step with the dev group. + +--- + +### C-52: the conformance fixture is keyed by algorithm, so broadening its coverage fails its own completeness test + +| Field | Value | +|-------|-------| +| ID | C-52 | +| Tier | 4 | +| Source | expert-review (2026-09-09, #94 review addendum) | +| Trigger | When a contributor adds a second real config for an algorithm already covered — e.g. `ParametricConflictology` with `family="zinb"` — `test_every_catalogued_algorithm_has_a_conformance_fixture` fails, because it asserts set equality between algorithm names and fixture keys | +| Location | `tests/test_config_conformance.py` (`MERGED_CONFIGS` keyed by algorithm; the equality assertion in `test_every_catalogued_algorithm_has_a_conformance_fixture`) | + +The completeness guard exists so the fixture set cannot silently shrink as the catalog grows. Keyed by algorithm name, it also caps the fixture set at exactly one config per algorithm — so the guard's shape punishes broadening it. views-models ships 29 baseline configs spanning 8 distinct `family x transform` combinations; this file covers 2. A constructor-level rejection introduced for an uncovered combination would ship green. **Discharge:** key `MERGED_CONFIGS` by a fixture label carrying its source model name, and assert that the set of algorithms *covered* is a superset of the catalog rather than equal to the fixture keys. + --- ## Disagreements diff --git a/tests/conftest.py b/tests/conftest.py index f985791..adc7bd7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,6 @@ "algorithm": "LocfModel", "level": "pgm", "time_steps": 36, - "targets": ["synth_target"], "regression_targets": ["synth_target"], "regression_point_metrics": ["MSE"], "prediction_format": "prediction_frame", diff --git a/tests/test_baseline.py b/tests/test_baseline.py index 3645c33..4354604 100644 --- a/tests/test_baseline.py +++ b/tests/test_baseline.py @@ -569,7 +569,7 @@ def test_mixture_predict_reproducible(mixture_df, partition_dict, targets): def test_catalog_get_zero_model(partition_dict, targets): from views_baseline.model.catalog import BaselineModelCatalog - config = {"targets": targets} + config = {"regression_targets": targets} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pgm") model = catalog.get_model("ZeroModel") assert isinstance(model, ZeroModel) @@ -578,7 +578,7 @@ def test_catalog_get_zero_model(partition_dict, targets): def test_catalog_get_average_model(partition_dict, targets): from views_baseline.model.catalog import BaselineModelCatalog - config = {"targets": targets, "window_months": 6} + config = {"regression_targets": targets, "window_months": 6} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pgm") model = catalog.get_model("AverageModel") assert isinstance(model, AverageModel) @@ -588,7 +588,7 @@ def test_catalog_get_average_model(partition_dict, targets): def test_catalog_unknown_model_raises(partition_dict, targets): from views_baseline.model.catalog import BaselineModelCatalog - config = {"targets": targets} + config = {"regression_targets": targets} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pgm") with pytest.raises(ValueError, match="NoSuchModel"): catalog.get_model("NoSuchModel") @@ -597,7 +597,7 @@ def test_catalog_unknown_model_raises(partition_dict, targets): def test_catalog_missing_required_key_raises(partition_dict, targets): from views_baseline.model.catalog import BaselineModelCatalog - config = {"targets": targets} # missing "window_months" required by AverageModel + config = {"regression_targets": targets} # missing "window_months" required by AverageModel catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pgm") with pytest.raises(ValueError, match="window_months"): catalog.get_model("AverageModel") @@ -606,7 +606,7 @@ def test_catalog_missing_required_key_raises(partition_dict, targets): def test_catalog_list_models(partition_dict, targets): from views_baseline.model.catalog import BaselineModelCatalog - config = {"targets": targets} + config = {"regression_targets": targets} catalog = BaselineModelCatalog(config=config, partition_dict=partition_dict, loa="pgm") names = catalog.list_models() expected = { diff --git a/tests/test_baseline_manager.py b/tests/test_baseline_manager.py index 5249240..ea74016 100644 --- a/tests/test_baseline_manager.py +++ b/tests/test_baseline_manager.py @@ -26,7 +26,7 @@ def test_manager_evaluate_uses_zero_model( "run_type": "eval", "level": "pgm", "algorithm": "ZeroModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -52,7 +52,7 @@ def test_manager_evaluate_uses_locf_model( "run_type": "eval", "level": "pgm", "algorithm": "LocfModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -84,7 +84,7 @@ def test_manager_forecast_uses_baseline_model( "run_type": "forecast", "level": "pgm", "algorithm": "LocfModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -117,7 +117,7 @@ def _frame_config(algorithm="LocfModel", run_type="forecast", targets=("y1", "y2 "run_type": run_type, "level": "pgm", "algorithm": algorithm, - "targets": list(targets), + "regression_targets": list(targets), "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -269,7 +269,7 @@ def test_manager_forecast_respects_algorithm_choice( "run_type": "forecast", "level": "pgm", "algorithm": "ZeroModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -282,7 +282,7 @@ def test_manager_forecast_respects_algorithm_choice( "run_type": "forecast", "level": "pgm", "algorithm": "LocfModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -307,7 +307,7 @@ def test_manager_setup_returns_model_and_data( "run_type": "eval", "level": "pgm", "algorithm": "ZeroModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -333,7 +333,7 @@ def test_manager_train_saves_artifact( "run_type": "calibration", "level": "pgm", "algorithm": "ZeroModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -371,7 +371,7 @@ def test_manager_evaluate_distributional_model( "run_type": "eval", "level": "pgm", "algorithm": "ConflictologyModel", - "targets": targets, + "regression_targets": targets, "window_months": 6, "n_samples": 64, "seed": 42, @@ -400,7 +400,7 @@ def test_manager_evaluate_sweep( "run_type": "eval", "level": "pgm", "algorithm": "LocfModel", - "targets": targets, + "regression_targets": targets, "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -429,7 +429,7 @@ def test_manager_forecast_distributional_model( "run_type": "forecast", "level": "pgm", "algorithm": "ConflictologyModel", - "targets": targets, + "regression_targets": targets, "window_months": 6, "n_samples": 64, "seed": 42, diff --git a/tests/test_catalog.py b/tests/test_catalog.py index 06741ac..a16f13c 100644 --- a/tests/test_catalog.py +++ b/tests/test_catalog.py @@ -11,7 +11,7 @@ def test_catalog_lists_all_models(): - config = {"targets": ["y1"], "window_months": 3} + config = {"regression_targets": ["y1"], "window_months": 3} partition_dict = {"test": (445, 492)} loa = "pg_id" @@ -28,7 +28,7 @@ def test_catalog_lists_all_models(): def test_catalog_returns_zero_model(): - config = {"targets": ["y1"]} + config = {"regression_targets": ["y1"]} partition_dict = {"test": (493, 540)} loa = "pg_id" @@ -36,13 +36,13 @@ def test_catalog_returns_zero_model(): model = catalog.get_model("ZeroModel") assert isinstance(model, ZeroModel) - assert model.targets == config["targets"] + assert model.targets == config["regression_targets"] assert model.partition_dict is partition_dict assert model.loa == loa def test_catalog_returns_locf_model(): - config = {"targets": ["y1", "y2"]} + config = {"regression_targets": ["y1", "y2"]} partition_dict = {"test": (445, 492)} loa = "pg_id" @@ -50,13 +50,13 @@ def test_catalog_returns_locf_model(): model = catalog.get_model("LocfModel") assert isinstance(model, LocfModel) - assert model.targets == config["targets"] + assert model.targets == config["regression_targets"] assert model.partition_dict is partition_dict assert model.loa == loa def test_catalog_returns_average_model(): - config = {"targets": ["y1"], "window_months": 6} + config = {"regression_targets": ["y1"], "window_months": 6} partition_dict = {"test": (445, 492)} loa = "pg_id" @@ -64,14 +64,14 @@ def test_catalog_returns_average_model(): model = catalog.get_model("AverageModel") assert isinstance(model, AverageModel) - assert model.targets == config["targets"] + assert model.targets == config["regression_targets"] assert model.partition_dict is partition_dict assert model.loa == loa assert model.window_months == config["window_months"] def test_catalog_returns_conflictology_model(): - config = {"targets": ["y1", "y2"], "window_months": 5, "n_samples": 128, "seed": 7} + config = {"regression_targets": ["y1", "y2"], "window_months": 5, "n_samples": 128, "seed": 7} partition_dict = {"test": (445, 492)} loa = "pg_id" @@ -79,7 +79,7 @@ def test_catalog_returns_conflictology_model(): model = catalog.get_model("ConflictologyModel") assert isinstance(model, ConflictologyModel) - assert model.targets == config["targets"] + assert model.targets == config["regression_targets"] assert model.partition_dict is partition_dict assert model.loa == loa assert model.window_months == config["window_months"] @@ -88,7 +88,7 @@ def test_catalog_returns_conflictology_model(): def test_catalog_missing_n_samples_for_conflictology_raises(): - config = {"targets": ["y1"], "window_months": 5} + config = {"regression_targets": ["y1"], "window_months": 5} partition_dict = {"test": (445, 492)} loa = "pg_id" @@ -98,7 +98,7 @@ def test_catalog_missing_n_samples_for_conflictology_raises(): def test_catalog_missing_keys_for_mixture_raises(): - config = {"targets": ["y1"]} + config = {"regression_targets": ["y1"]} partition_dict = {"test": (445, 492)} loa = "pg_id" @@ -108,7 +108,7 @@ def test_catalog_missing_keys_for_mixture_raises(): def test_catalog_raises_for_unknown_model(): - config = {"targets": ["y1"]} + config = {"regression_targets": ["y1"]} partition_dict = {"test": (493, 540)} loa = "pg_id" @@ -120,7 +120,7 @@ def test_catalog_raises_for_unknown_model(): def test_catalog_returns_mixture_model(): config = { - "targets": ["y1"], "window_months": 18, "lambda_mix": 0.05, + "regression_targets": ["y1"], "window_months": 18, "lambda_mix": 0.05, "n_samples": 256, "seed": 11, } partition_dict = {"test": (493, 540)} @@ -130,7 +130,7 @@ def test_catalog_returns_mixture_model(): model = catalog.get_model("MixtureBaseline") assert isinstance(model, MixtureBaseline) - assert model.targets == config["targets"] + assert model.targets == config["regression_targets"] assert model.partition_dict is partition_dict assert model.loa == loa assert model.window_months == 18 @@ -141,7 +141,7 @@ def test_catalog_returns_mixture_model(): def test_catalog_returns_parametric_conflictology(): config = { - "targets": ["y1"], "window_months": 5, "n_samples": 128, + "regression_targets": ["y1"], "window_months": 5, "n_samples": 128, "seed": 7, "family": "nb", "transform": "none", } catalog = BaselineModelCatalog( @@ -159,7 +159,7 @@ def test_catalog_returns_parametric_conflictology(): def test_catalog_returns_parametric_hurdle(): config = { - "targets": ["y1", "y2"], "window_months": 9, "n_samples": 256, + "regression_targets": ["y1", "y2"], "window_months": 9, "n_samples": 256, "seed": 11, "family": "gumbel", "transform": "log1p", } catalog = BaselineModelCatalog( @@ -176,7 +176,8 @@ def test_catalog_returns_parametric_hurdle(): def test_catalog_missing_family_for_parametric_raises(): config = { - "targets": ["y1"], "window_months": 5, "n_samples": 128, "seed": 7, "transform": "none", + "regression_targets": ["y1"], "window_months": 5, "n_samples": 128, "seed": 7, + "transform": "none", } catalog = BaselineModelCatalog( config=config, partition_dict={"test": (445, 492)}, loa="pg_id" @@ -188,7 +189,7 @@ def test_catalog_missing_family_for_parametric_raises(): def test_catalog_illegal_family_transform_fails_loud(): """ADR-021/ADR-022: nb + log1p is a contract violation, surfaced at construction.""" config = { - "targets": ["y1"], "window_months": 5, "n_samples": 128, + "regression_targets": ["y1"], "window_months": 5, "n_samples": 128, "seed": 7, "family": "nb", "transform": "log1p", } catalog = BaselineModelCatalog( @@ -200,7 +201,7 @@ def test_catalog_illegal_family_transform_fails_loud(): def test_catalog_continuous_family_rejected_by_no_hurdle_via_catalog(): config = { - "targets": ["y1"], "window_months": 5, "n_samples": 128, + "regression_targets": ["y1"], "window_months": 5, "n_samples": 128, "seed": 7, "family": "lognormal", "transform": "none", } catalog = BaselineModelCatalog( @@ -212,7 +213,7 @@ def test_catalog_continuous_family_rejected_by_no_hurdle_via_catalog(): def test_catalog_count_family_rejected_by_hurdle_via_catalog(): config = { - "targets": ["y1"], "window_months": 5, "n_samples": 128, + "regression_targets": ["y1"], "window_months": 5, "n_samples": 128, "seed": 7, "family": "nb", "transform": "none", } catalog = BaselineModelCatalog( @@ -238,7 +239,7 @@ def test_catalog_forwards_all_config_params(algo, cls): # Distinct, non-default values for every possible constructor param. config = { - "targets": ["y1"], + "regression_targets": ["y1"], "window_months": 7, "n_samples": 13, "lambda_mix": 0.3, @@ -250,13 +251,19 @@ def test_catalog_forwards_all_config_params(algo, cls): # partition_dict and loa are injected by the catalog itself, not from config. injected = {"self", "partition_dict", "loa"} + # Constructor params whose config key has a different name. Without this map the + # rename to `regression_targets` made `pname not in config` true for `targets`, so + # this guard silently stopped checking the one parameter the C-10 class is about + # while still reporting green (#94 review, finding 1). + config_key = {"targets": "regression_targets"} for pname in inspect.signature(cls.__init__).parameters: - if pname in injected or pname not in config: + key = config_key.get(pname, pname) + if pname in injected or key not in config: continue - assert getattr(model, pname) == config[pname], ( - f"{algo}: constructor param '{pname}' was not forwarded from config " - f"(got {getattr(model, pname)!r}, expected {config[pname]!r}) — a silently " - f"dropped parameter (C-10 class)." + assert getattr(model, pname) == config[key], ( + f"{algo}: constructor param '{pname}' was not forwarded from config key " + f"'{key}' (got {getattr(model, pname)!r}, expected {config[key]!r}) — a " + f"silently dropped parameter (C-10 class)." ) @@ -273,7 +280,7 @@ def test_catalog_forwards_all_config_params_parametric(algo, cls, family): import inspect config = { - "targets": ["y1"], + "regression_targets": ["y1"], "window_months": 7, "n_samples": 13, "seed": 99, @@ -286,11 +293,37 @@ def test_catalog_forwards_all_config_params_parametric(algo, cls, family): model = catalog.get_model(algo) injected = {"self", "partition_dict", "loa"} + # Constructor params whose config key has a different name. Without this map the + # rename to `regression_targets` made `pname not in config` true for `targets`, so + # this guard silently stopped checking the one parameter the C-10 class is about + # while still reporting green (#94 review, finding 1). + config_key = {"targets": "regression_targets"} for pname in inspect.signature(cls.__init__).parameters: - if pname in injected or pname not in config: + key = config_key.get(pname, pname) + if pname in injected or key not in config: continue - assert getattr(model, pname) == config[pname], ( - f"{algo}: constructor param '{pname}' was not forwarded from config " - f"(got {getattr(model, pname)!r}, expected {config[pname]!r}) — a silently " - f"dropped parameter (C-10 class)." + assert getattr(model, pname) == config[key], ( + f"{algo}: constructor param '{pname}' was not forwarded from config key " + f"'{key}' (got {getattr(model, pname)!r}, expected {config[key]!r}) — a " + f"silently dropped parameter (C-10 class)." ) + + +def test_list_models_needs_no_targets_in_config(): + """`list_models()` is a Stable, side-effect-free accessor (ADR-004; the CIC). + + Deriving targets in `__init__` made enumerating the catalog raise + `KeyError: 'regression_targets'` — a precondition no caller enumerating algorithms + has reason to satisfy (#94 review, finding 3/A3). + """ + catalog = BaselineModelCatalog(config={}, partition_dict={}, loa="pgm") + assert "ZeroModel" in catalog.list_models() + + +def test_catalog_rejects_a_scalar_target_string(): + """A bare string would `list()` into one target per character (#94 review, finding 11).""" + catalog = BaselineModelCatalog( + config={"regression_targets": "lr_ged_sb"}, partition_dict={}, loa="pgm" + ) + with pytest.raises(ValueError, match="sequence of target names"): + catalog.get_model("ZeroModel") diff --git a/tests/test_config_conformance.py b/tests/test_config_conformance.py new file mode 100644 index 0000000..f6a531d --- /dev/null +++ b/tests/test_config_conformance.py @@ -0,0 +1,267 @@ +"""Conformance guard (issue #85 / #84): every catalog builder must construct from a +**realistic merged views-models config**, not from a hand-rolled test fixture. + +This is the test that did not exist on 2026-08-02, and whose absence let every baseline +model stay dead for five weeks. + +**The mechanism it guards.** views-pipeline-core retired the synthesised ``targets`` config +key (``507ae11``, refs #380/#381, shipped in 3.0.0) and migrated its own 13 read sites. +views-baseline was never swept, so ``catalog.py`` kept reading ``config["targets"]`` and +every ``get_model()`` raised ``KeyError: 'targets'``. The suite stayed green throughout +because ``conftest.MANAGER_BASE_CONFIG`` manufactured the retired key — the fixture, not +the dependency, defined the contract. + +**Why the check belongs here and not upstream.** pipeline-core has registered the blind +spot itself (its C-289: a repo "cannot see a **consumer**, because being imported by +someone leaves no trace in your own source") and its conformance suite explicitly exempts +views-baseline, pushing the check to our side. `reproducibility_gate` already states the +intended direction — downstream repos import *us* to validate their configs — so the +matching obligation is that we validate against *their* config shape. + +**Fixture provenance.** The configs below are copied from the shipped views-models +baselines, merged the way ``ConfigurationManager.get_combined_config`` merges them +(``config_hyperparameters`` then ``config_meta``, meta winning on collision), as of +**2026-09-09**: + +* ``zero_pgmbaseline`` / ``locf_pgmbaseline`` / ``average_pgmbaseline`` — the point models +* ``light_strider`` (ConflictologyModel), ``black_ranger`` (MixtureBaseline) +* ``doctorish_dwarf`` (ParametricConflictology), ``bashful_dwarf`` (ParametricHurdle) + +They are **copied, not imported**: nothing in this repo may read across the views-models +boundary. The cost of the copy is that it can drift from the live configs; refresh it when +views-models changes a baseline config's shape. It catches a *key rename* — the failure +that actually happened — not a *value* change. + +**What this does not cover.** It constructs models; it does not fit or predict them, and +it does not verify that pipeline-core would accept these dicts (that would need the real +``ConfigurationManager``). Those belong to the runtime smoke test in views-models. +""" + +import pytest + +from views_baseline.infrastructure.reproducibility_gate import ReproducibilityGate +from views_baseline.model.catalog import BaselineModelCatalog + +# Keys views-evaluation retired in 0.4.0; pipeline-core's `combined_targets()` RAISES on a +# config carrying any of them. A fixture containing one is not a realistic merged config — +# which is exactly how the old conftest fixture drifted out of contact with production. +_RETIRED_EVALUATION_KEYS = ( + "targets", + "metrics", + "regression_uncertainty_metrics", + "classification_uncertainty_metrics", +) + +_PARTITION = {"test": (493, 540)} + +# Core keys every shipped baseline config carries, verified across all 29 on 2026-09-09. +# +# This block is sized to pipeline-core's `CoreConfigSniffer`, not to CORE_GENOME. An +# earlier version carried only the five genome keys, which made the module docstring's +# claim ("configs pipeline-core would accept") false: the sniffer runs as the first +# statement of `ModelManager.execute_single_run` and rejects a config missing `name`, +# `creator` or `rolling_origin_stride`, plus a regression metric key. A fixture trimmed +# to our own genome is a hand-roll, and the whole point of this file is not to build the +# guard on a hand-roll (#94 review, finding 7). +_CORE = { + # CORE_GENOME + "steps": [*range(1, 37)], + "time_steps": 36, + "prediction_format": "prediction_frame", + "level": "pgm", + # additionally required by CoreConfigSniffer.MANDATORY_KEYS_UNIVERSAL/_MODEL + "name": "conformance_fixture", + "creator": "views-baseline conformance suite", + "rolling_origin_stride": 1, + # from config_deployment.py. Kept on the LEGACY key rather than ADR-057's `maturity` + # because all 29 shipped baseline configs are still on it — the fixture is supposed to + # mirror what really ships, not what pipeline-core would prefer. It warns, and the + # sniffer reads it as maturity='candidate'. Revisit when views-models migrates. + "deployment_status": "shadow", + # required by the sniffer whenever prediction_format == "prediction_frame" + "skip_predictions_delivery": True, +} + +MERGED_CONFIGS = { + # zero_pgmbaseline / locf_pgmbaseline — no algorithm-specific keys + "ZeroModel": { + **_CORE, "algorithm": "ZeroModel", + "regression_targets": ["lr_ged_sb"], "regression_point_metrics": ["MSE"], + }, + "LocfModel": { + **_CORE, "algorithm": "LocfModel", + "regression_targets": ["lr_ged_sb"], "regression_point_metrics": ["MSE"], + }, + # average_pgmbaseline + "AverageModel": { + **_CORE, + "algorithm": "AverageModel", + "regression_targets": ["lr_ged_sb"], + "regression_point_metrics": ["MSE"], + "window_months": 18, + }, + # light_strider + "ConflictologyModel": { + **_CORE, + "algorithm": "ConflictologyModel", + "regression_targets": ["lr_sb_best", "lr_ns_best", "lr_os_best"], + "regression_sample_metrics": ["twCRPS"], + "window_months": 36, + "n_samples": 64, + "seed": 42, + }, + # black_ranger + "MixtureBaseline": { + **_CORE, + "algorithm": "MixtureBaseline", + "regression_targets": ["lr_os_best"], + "regression_sample_metrics": ["twCRPS"], + "window_months": 18, + "lambda_mix": 0.05, + "n_samples": 256, + "seed": 42, + }, + # doctorish_dwarf — native-zero family, no transform + "ParametricConflictology": { + **_CORE, + "algorithm": "ParametricConflictology", + "regression_targets": ["lr_sb_best", "lr_ns_best", "lr_os_best"], + "regression_sample_metrics": ["twCRPS"], + "window_months": 36, + "n_samples": 64, + "seed": 42, + "family": "nb", + "transform": "none", + }, + # bashful_dwarf — continuous positive-part family with log1p + "ParametricHurdleConflictology": { + **_CORE, + "algorithm": "ParametricHurdleConflictology", + "regression_targets": ["lr_sb_best", "lr_ns_best", "lr_os_best"], + "regression_sample_metrics": ["twCRPS"], + "window_months": 36, + "n_samples": 64, + "seed": 42, + "family": "gamma", + "transform": "log1p", + }, +} + + +def test_every_catalogued_algorithm_has_a_conformance_fixture(): + """A new algorithm must arrive with a real config, or this guard silently shrinks.""" + catalogued = set( + BaselineModelCatalog( + config=MERGED_CONFIGS["ZeroModel"], partition_dict=_PARTITION, loa="pgm" + ).list_models() + ) + assert catalogued == set(MERGED_CONFIGS), ( + f"catalog and conformance fixtures disagree: " + f"only in catalog={sorted(catalogued - set(MERGED_CONFIGS))}, " + f"only in fixtures={sorted(set(MERGED_CONFIGS) - catalogued)}. " + f"Add the new algorithm's real merged config here (ADR-012 step)." + ) + + +@pytest.mark.parametrize("algorithm", sorted(MERGED_CONFIGS)) +def test_fixture_carries_no_retired_evaluation_key(algorithm): + """The fixtures must be configs pipeline-core would accept (#380). + + `combined_targets()` raises on any retired key, so a fixture carrying one could never + reach the catalog in production — and a guard built on it proves nothing. This is the + property the old `conftest.MANAGER_BASE_CONFIG` violated. + """ + present = [k for k in _RETIRED_EVALUATION_KEYS if k in MERGED_CONFIGS[algorithm]] + assert not present, ( + f"{algorithm}: conformance fixture carries retired evaluation key(s) {present}. " + f"pipeline-core's combined_targets() raises on these, so this is not a config any " + f"real run could produce." + ) + + +def test_shared_test_fixtures_carry_no_retired_evaluation_key(): + """The incident fixture itself must stay clean (#94 review, finding 9). + + `MANAGER_BASE_CONFIG` in conftest is the dict that manufactured `targets` for five + weeks. This PR deletes the line; nothing stopped it coming back. Re-adding a retired + key there to make some legacy manager test pass — the same one-line convenience that + created #84 — would otherwise leave every guard in this repo green. + """ + from conftest import MANAGER_BASE_CONFIG + + present = [k for k in _RETIRED_EVALUATION_KEYS if k in MANAGER_BASE_CONFIG] + assert not present, ( + f"conftest.MANAGER_BASE_CONFIG carries retired evaluation key(s) {present}. " + f"pipeline-core's combined_targets() raises on these — this is the exact fixture " + f"drift that caused #84." + ) + + +@pytest.mark.parametrize("algorithm", sorted(MERGED_CONFIGS)) +def test_real_config_passes_the_reproducibility_gate(algorithm): + """A shipped config must satisfy CORE_GENOME + its ALGORITHM_GENOME (#87).""" + ReproducibilityGate.Config.audit_manifest(MERGED_CONFIGS[algorithm]) + + +@pytest.mark.parametrize("algorithm", sorted(MERGED_CONFIGS)) +def test_real_config_constructs_its_model(algorithm): + """The regression this file exists for: `KeyError: 'targets'` at catalog.py:75. + + Reverting the `regression_targets` migration in `catalog.py` must fail here. + """ + config = MERGED_CONFIGS[algorithm] + catalog = BaselineModelCatalog( + config=config, partition_dict=_PARTITION, loa=config["level"] + ) + model = catalog.get_model(algorithm) + + assert type(model).__name__ == algorithm + assert model.targets == config["regression_targets"], ( + f"{algorithm}: model targets {model.targets} do not match the config's " + f"regression_targets {config['regression_targets']}." + ) + assert model.loa == config["level"] + + +@pytest.mark.parametrize("algorithm", sorted(MERGED_CONFIGS)) +def test_catalog_does_not_alias_the_config_target_list(algorithm): + """The model's target list must not be the config's list object. + + A shared list would let a model mutate the config that other models are still being + built from — the kind of implicit coupling that is invisible until two models disagree. + """ + config = MERGED_CONFIGS[algorithm] + catalog = BaselineModelCatalog( + config=config, partition_dict=_PARTITION, loa=config["level"] + ) + model = catalog.get_model(algorithm) + assert model.targets is not config["regression_targets"] + + +@pytest.mark.parametrize("algorithm", sorted(MERGED_CONFIGS)) +def test_fixture_is_accepted_by_pipeline_cores_own_sniffer(algorithm): + """The fixtures must be configs pipeline-core would actually run (#94 review, #7). + + `test_fixture_carries_no_retired_evaluation_key` compares one module literal against + another and so cannot fail on any change to this package — it is fixture hygiene, not + evidence. This test is the one that can fail: it hands each fixture to + `CoreConfigSniffer`, which pipeline-core runs as the first statement of + `ModelManager.execute_single_run`. If a fixture drifts into a shape no real run could + produce, the conformance guard above is being built on a hand-roll — which is the + precise mistake that let #84 survive. + + Skipped where pipeline-core is absent; it is the only test here that needs it. + """ + sniffer = pytest.importorskip( + "views_pipeline_core.modules.validation.core_config_sniffer" + ) + # The sniffer takes the OUTER partition dict, keyed by run_type, whose values are + # {"train": (a, b), "test": (c, d)}. `_PARTITION` above is the inner one — the shape + # views-baseline actually receives (`self._data_loader.partition_dict`), which is why + # models read `partition_dict["test"]` directly. Test span must be + # time_steps + MAX_SHIFT_COUNT = 36 + 12 = 48 months. + sniffer.CoreConfigSniffer( + MERGED_CONFIGS[algorithm], + {"calibration": {"train": (121, 396), "test": (397, 444)}}, + target="model", + ).sniff_all("calibration") diff --git a/tests/test_falsification_seed_wiring.py b/tests/test_falsification_seed_wiring.py index 80ff5cd..e3434bd 100644 --- a/tests/test_falsification_seed_wiring.py +++ b/tests/test_falsification_seed_wiring.py @@ -35,7 +35,7 @@ def _df(): def _catalog(seed): cfg = { - "targets": ["y1"], + "regression_targets": ["y1"], "window_months": 6, "n_samples": 16, "lambda_mix": 0.5, @@ -73,7 +73,7 @@ def test_different_seed_produces_different_output(algo): @pytest.mark.parametrize("algo", ["ConflictologyModel", "MixtureBaseline"]) def test_catalog_requires_seed(algo): """ADR-021: the catalog must NOT silently default seed — a config without it fails loud.""" - cfg = {"targets": ["y1"], "window_months": 6, "n_samples": 8, "lambda_mix": 0.5} + cfg = {"regression_targets": ["y1"], "window_months": 6, "n_samples": 8, "lambda_mix": 0.5} cat = BaselineModelCatalog(config=cfg, partition_dict=_PARTITION, loa="pgm") with pytest.raises(ValueError, match="seed"): cat.get_model(algo) diff --git a/tests/test_reproducibility_gate.py b/tests/test_reproducibility_gate.py index c56f1ab..d79dd5c 100644 --- a/tests/test_reproducibility_gate.py +++ b/tests/test_reproducibility_gate.py @@ -15,6 +15,77 @@ def test_core_genome_is_list_of_strings(): assert len(genome) > 0 +# Keys whose absence must be a *named contract violation*, with the reason each one +# earns that. Asserted by name, deliberately: a test parametrized over CORE_GENOME +# cannot catch a key being removed from it (the parametrize list simply shrinks and the +# case stops running) — which is exactly how mutations M5/M6 survived the #89 audit with +# the whole suite green. +# +# The reasons are not uniform, and an earlier version of this dict wrongly claimed they +# were (#94 review). Two distinct failure modes: +# +# * dereferenced unconditionally -> absence is a bare KeyError past a passing audit, +# the #84 shape; +# * read with a silent default -> absence is not an error at all, which is worse: +# `prediction_format` is never read in this package, and pipeline-core reads it as +# `.get("prediction_format", "dataframe")`, so omitting it silently routes a +# PredictionFrame-only model down the dataframe emission path. +# +# NOT in this dict, and deliberately: `run_type`, which `_train_model_artifact` reads one +# line after the audit passes and which pipeline-core lists in its own +# `_SAFETY_CRITICAL_KEYS`. It is injected at runtime rather than declared in a config +# file, so requiring it here would break the static downstream validation this gate +# advertises (see C-48). Tracked, not silently absorbed. +_KEYS_DEREFERENCED_ON_EVERY_RUN = { + "steps": "dereferenced — pipeline-core evaluation/forecast horizon", + "time_steps": "dereferenced — manager._generate_predictions output_length", + "prediction_format": "silent default — pipeline-core .get(..., 'dataframe')", + "regression_targets": "dereferenced — BaselineModelCatalog.targets (all 7 builders)", + "level": "dereferenced — manager._setup_model_and_data -> loa", +} + + +def test_core_genome_declares_every_unconditionally_read_key(): + """Removing a key from CORE_GENOME must break a test, not just production (#85).""" + genome = set(ReproducibilityGate.Config.CORE_GENOME) + missing = {k: why for k, why in _KEYS_DEREFERENCED_ON_EVERY_RUN.items() if k not in genome} + assert not missing, ( + f"CORE_GENOME no longer declares {sorted(missing)}. Each is read unconditionally " + f"on every run ({missing}), so dropping it from the genome restores the failure " + f"mode of #84: the gate passes and the run dies on a bare KeyError." + ) + + +@pytest.mark.parametrize("missing_key", sorted(_KEYS_DEREFERENCED_ON_EVERY_RUN)) +def test_each_core_key_is_enforced_not_merely_listed(missing_key): + """A declared core key must actually be audited — listing is not enforcing. + + Asserts only that the absence is rejected *loudly*, not which of `audit_manifest`'s + four checks fires. The #89 guard audit established that checks 1 and 4 overlap: a + missing key reaches check 4 as `config.get(k) is None`, so it is rejected either way + and only the message differs ("Missing core parameters" vs "set to None"). Mutation + M10 — exempting a key from check 1 — survives for that reason and is an equivalent + mutant, not a gap. + """ + config = { + "algorithm": "ZeroModel", + "steps": [*range(1, 37)], + "time_steps": 36, + "prediction_format": "prediction_frame", + "regression_targets": ["y1"], + "level": "pgm", + } + del config[missing_key] + # Exact list, not `re.escape(missing_key)`: pytest's `match` is re.search, so + # "steps" also matches a message naming only "time_steps" — the same substring + # looseness this file removes 100 lines below, re-introduced (#94 review, finding 5). + with pytest.raises( + MissingHyperparameterError, + match=rf"Missing core parameters: \['{missing_key}'\]", + ): + ReproducibilityGate.Config.audit_manifest(config) + + def test_algorithm_genomes_covers_all_catalog_models(): expected = { "ZeroModel", "LocfModel", "AverageModel", @@ -34,7 +105,8 @@ def test_parametric_genomes_require_family_transform_seed(): def test_audit_manifest_accepts_valid_zero_model_config(): config = { "algorithm": "ZeroModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -45,7 +117,8 @@ def test_audit_manifest_accepts_valid_zero_model_config(): def test_audit_manifest_accepts_valid_mixture_config(): config = { "algorithm": "MixtureBaseline", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -61,7 +134,8 @@ def test_audit_manifest_rejects_missing_seed_for_distributional(): """ADR-021: `seed` is a required, audited genome key for distributional models.""" config = { "algorithm": "ConflictologyModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -76,7 +150,8 @@ def test_audit_manifest_rejects_missing_seed_for_distributional(): def test_audit_manifest_accepts_valid_parametric_config(): config = { "algorithm": "ParametricConflictology", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -94,7 +169,8 @@ def test_audit_manifest_rejects_missing_parametric_key(missing): """ADR-022: family/transform/seed are audited — omitting any one fails loud.""" config = { "algorithm": "ParametricHurdleConflictology", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -112,18 +188,27 @@ def test_audit_manifest_rejects_missing_parametric_key(missing): def test_audit_manifest_rejects_missing_core_key(): config = { "algorithm": "ZeroModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "time_steps": 36, - # "steps" is missing + "prediction_format": "prediction_frame", + # "steps" is missing — and only "steps", so the assertion below can name the + # exact missing list. This config previously also omitted "prediction_format" + # while its comment claimed otherwise; the loose match="steps" hid that. } - with pytest.raises(MissingHyperparameterError, match="steps"): + # Asserted as the exact missing list, not `match="steps"`: that substring also + # matches "time_steps", and after the #85 CORE_GENOME promotion it would match a + # message naming keys this test says nothing about. A guard that can pass for a + # reason other than the one it is named for is not a guard. + with pytest.raises(MissingHyperparameterError, match=r"Missing core parameters: \['steps'\]"): ReproducibilityGate.Config.audit_manifest(config) def test_audit_manifest_rejects_missing_algorithm_key(): config = { "algorithm": "AverageModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -136,7 +221,8 @@ def test_audit_manifest_rejects_missing_algorithm_key(): def test_audit_manifest_rejects_unknown_algorithm(): config = { "algorithm": "NonExistentModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -169,7 +255,7 @@ def test_manager_gate_rejects_incomplete_config( "run_type": "eval", "level": "pg_id", "algorithm": "ZeroModel", - "targets": ["y1"], + "regression_targets": ["y1"], "prediction_format": "prediction_frame", # "steps" and "time_steps" are missing } @@ -177,7 +263,12 @@ def test_manager_gate_rejects_incomplete_config( mgr = make_manager(config, manager_partition_dict) monkeypatch.setattr(bm, "read_dataframe", lambda path: manager_df) - with pytest.raises(MissingHyperparameterError, match="steps"): + # Exact missing list, for the same reason as above: "steps" alone would also be + # satisfied by a message naming only "time_steps". + with pytest.raises( + MissingHyperparameterError, + match=r"Missing core parameters: \['steps', 'time_steps'\]", + ): mgr._setup_model_and_data() @@ -202,7 +293,8 @@ def test_none_value_injection(): """A required key present but set to None must be rejected.""" config = { "algorithm": "AverageModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": None, "prediction_format": "prediction_frame", @@ -216,7 +308,8 @@ def test_empty_string_algorithm(): """An empty-string algorithm must be rejected as unknown.""" config = { "algorithm": "", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -228,7 +321,8 @@ def test_empty_string_algorithm(): def test_missing_algorithm_key(): """Config with no 'algorithm' key at all must be rejected explicitly.""" config = { - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -241,7 +335,8 @@ def test_extra_keys_ignored(): """Surplus keys in the config must not cause errors.""" config = { "algorithm": "ZeroModel", - "targets": ["y1"], + "regression_targets": ["y1"], + "level": "pgm", "steps": [*range(1, 37)], "time_steps": 36, "prediction_format": "prediction_frame", @@ -249,3 +344,37 @@ def test_extra_keys_ignored(): "another_extra": 999, } ReproducibilityGate.Config.audit_manifest(config) + + +def test_audit_manifest_rejects_empty_required_value(): + """An empty list declares nothing (#94 review, finding 2). + + `regression_targets: []` passed every check before: presence yes, non-None yes. The + run then built every model with no targets and reported success having written no + PredictionFrames. It is reachable in production precisely because check 4 is not: + pipeline-core's `get_combined_config` rewrites `regression_targets: None` to `[]` + before the manager sees it. + """ + config = { + "algorithm": "ZeroModel", + "steps": [*range(1, 37)], + "time_steps": 36, + "prediction_format": "prediction_frame", + "regression_targets": [], + "level": "pgm", + } + with pytest.raises(MissingHyperparameterError, match=r"empty: \['regression_targets'\]"): + ReproducibilityGate.Config.audit_manifest(config) + + +def test_audit_manifest_accepts_scalar_required_values(): + """The emptiness check must not trip on ints — they have no length.""" + config = { + "algorithm": "ZeroModel", + "steps": [*range(1, 37)], + "time_steps": 36, + "prediction_format": "prediction_frame", + "regression_targets": ["y1"], + "level": "pgm", + } + ReproducibilityGate.Config.audit_manifest(config) diff --git a/views_baseline/infrastructure/reproducibility_gate.py b/views_baseline/infrastructure/reproducibility_gate.py index 77a6433..3660d69 100644 --- a/views_baseline/infrastructure/reproducibility_gate.py +++ b/views_baseline/infrastructure/reproducibility_gate.py @@ -21,7 +21,22 @@ class Config: """Gates related to configuration and hyperparameter integrity.""" # Core keys required by ALL baseline models regardless of algorithm. - CORE_GENOME = ["steps", "time_steps", "prediction_format"] + # + # `regression_targets` and `level` were promoted here after the pipeline-core + # #380 outage (issue #85): both are dereferenced unconditionally on every run — + # `level` at `manager/baseline_manager.py` one line after this audit returns, and + # `regression_targets` by all seven catalog factories — but neither was declared, + # so a config missing one cleared both validation layers and died on a bare + # KeyError deep in a factory. ADR-009 recorded that gap as accepted debt in March + # 2026; it is what the outage was made of. Verified against all 29 shipped + # baseline configs before promotion (29/29 declare both). + CORE_GENOME = [ + "steps", + "time_steps", + "prediction_format", + "regression_targets", + "level", + ] # Algorithm-specific keys (audited only when the algorithm matches). ALGORITHM_GENOMES = { @@ -110,3 +125,26 @@ def audit_manifest(config: dict) -> None: ) logger.error(msg) raise MissingHyperparameterError(msg) + + # 5. Reject empty values for all required keys. + # + # An empty list is the same claim as None — "declared, but says nothing" — + # and it arrives by a path None cannot: pipeline-core's + # `get_combined_config` rewrites `regression_targets: None` to `[]` before + # the manager ever sees it, so check 4 is unreachable in-pipeline for + # exactly the key most worth guarding. Without this, an empty + # `regression_targets` passes the whole gate, every model is built with no + # targets, and the run reports success having written no PredictionFrames + # at all. Scalars (`time_steps`, `seed`) have no length and are skipped. + empties = [ + k for k in all_required + if hasattr(config.get(k), "__len__") and len(config[k]) == 0 + ] + if empties: + msg = ( + "REPRODUCIBILITY CONTRACT VIOLATED: " + f"Mandatory parameters are empty: {empties}. " + "An empty value declares nothing; implicit defaults are forbidden." + ) + logger.error(msg) + raise MissingHyperparameterError(msg) diff --git a/views_baseline/model/catalog.py b/views_baseline/model/catalog.py index 42e4646..25db8bc 100644 --- a/views_baseline/model/catalog.py +++ b/views_baseline/model/catalog.py @@ -16,6 +16,18 @@ class BaselineModelCatalog: def __init__(self, config: dict, partition_dict: dict, loa: str): """ Catalog of available baseline models. + + Reads the views-pipeline-core config vocabulary: ``regression_targets``. + (``targets`` was a synthesised backward-compatibility key, retired in + pipeline-core #380; ``combined_targets()`` now *raises* on a config that + still carries it.) Derived once here rather than at each of the seven + factory sites, so a future rename of the platform's target vocabulary is + one edit, not seven. + + ``classification_targets`` is deliberately not read: no baseline is wired + for a classification target today, and no baseline config declares one. + See ADR-012 when that changes — the decision is per-model (Zero/LOCF carry + over cleanly; the magnitude-fitting families do not), not catalog-wide. """ self.config = config self.partition_dict = partition_dict @@ -31,6 +43,27 @@ def __init__(self, config: dict, partition_dict: dict, loa: str): "ParametricHurdleConflictology": self._get_parametric_hurdle, } + @property + def targets(self) -> list: + """The target names, read from the one config key that carries them. + + Derived on access rather than stored in ``__init__`` so that + :meth:`list_models` stays a pure accessor with no config precondition — it is + declared Stable in ADR-004 and "no side effects" in this class's CIC, and + enumerating the catalog must not require a config that carries targets. + + Rejects a bare string: ``regression_targets="lr_ged_sb"`` would otherwise + ``list()`` into nine single-character targets, which both the gate (presence and + non-emptiness only) and the models accept without complaint. + """ + raw = self.config["regression_targets"] + if isinstance(raw, str): + raise ValueError( + f"regression_targets must be a sequence of target names, got the string " + f"{raw!r}. A bare string would be split into one target per character." + ) + return list(raw) + def get_model(self, model_name: str): """ Returns an initialized model instance. @@ -54,17 +87,17 @@ def list_models(self): def _get_zero_model(self): return ZeroModel( - targets=self.config["targets"], partition_dict=self.partition_dict, loa=self.loa + targets=self.targets, partition_dict=self.partition_dict, loa=self.loa ) def _get_locf_model(self): return LocfModel( - targets=self.config["targets"], partition_dict=self.partition_dict, loa=self.loa + targets=self.targets, partition_dict=self.partition_dict, loa=self.loa ) def _get_average_model(self): return AverageModel( - targets=self.config["targets"], + targets=self.targets, window_months=self.config["window_months"], partition_dict=self.partition_dict, loa=self.loa, @@ -72,7 +105,7 @@ def _get_average_model(self): def _get_conflictology_model(self): return ConflictologyModel( - targets=self.config["targets"], + targets=self.targets, window_months=self.config["window_months"], partition_dict=self.partition_dict, loa=self.loa, @@ -82,7 +115,7 @@ def _get_conflictology_model(self): def _get_mixture_model(self): return MixtureBaseline( - targets=self.config["targets"], + targets=self.targets, window_months=self.config["window_months"], lambda_mix=self.config["lambda_mix"], n_samples=self.config["n_samples"], @@ -95,7 +128,7 @@ def _get_parametric_conflictology(self): # family/transform/seed are required, audited genome keys (ADR-021/ADR-022); the # constructor fails loud on an unsupported family or an illegal family×transform. return ParametricConflictology( - targets=self.config["targets"], + targets=self.targets, window_months=self.config["window_months"], partition_dict=self.partition_dict, loa=self.loa, @@ -109,7 +142,7 @@ def _get_parametric_hurdle(self): # family/transform/seed are required, audited genome keys (ADR-021/ADR-022); the # constructor fails loud on a non-continuous family or an illegal family×transform. return ParametricHurdleConflictology( - targets=self.config["targets"], + targets=self.targets, window_months=self.config["window_months"], partition_dict=self.partition_dict, loa=self.loa,