diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc646e..c0142fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,116 @@ All notable changes to `views-frames` are documented here. The format is based o [Keep a Changelog](https://keepachangelog.com/), and this project adheres to [Semantic Versioning](https://semver.org/) as governed in `GOVERNANCE.md`. +## [2.0.0] — 2026-08-18 + +**A frame's index must actually be an index.** Until now you could hand a frame anything as +its `index` — a different frame, or a bare object with an `n_rows` attribute — and it was +accepted. The frame was built, it looked fine, and the published conformance suite said it +was fine. That now raises `TypeError`. + +**If you pass a `SpatioTemporalIndex`, you will not notice this release.** That is what the +type hint has always said and what every example does. Change your version constraint, +re-lock, and carry on: + +```diff +- views-frames = ">=1.10.2,<2" ++ views-frames = ">=2.0.0,<3" +``` + +This is the package's **first MAJOR** and the first move of `CONFORMANCE_FLOOR` since the +v1.0.0 freeze (`1.0.0` → `2.0.0`). The decision, the full migration table and the reasoning +are in **ADR-028**. + +### What actually breaks + +| If your code… | Before | Now | +|---|---|---| +| passes a `SpatioTemporalIndex` as `index` | works | **works, unchanged** | +| passes a frame or other object as `index` | silently constructed | `TypeError` | +| passes a non-index to `reindex`/`reindex_fill` | `AttributeError` on `_level` | `TypeError`, naming what was expected | +| mutates `frame.values` in place | silently corrupted buffer-sharing frames | `ValueError: assignment destination is read-only` | +| calls `map_estimate` on `inf`/`NaN` draws | bare `IndexError` | `ValueError` naming the cause | + +Only the fourth row can plausibly stop working code, and ADR-025 already documented that +operation as unsupported. + +**No `from_legacy_*` shim.** GOVERNANCE's MAJOR process asks for one where a consumer format +changes. Nothing here changes a wire format, a serialized layout or a signature — only the +set of inputs that were never valid narrows, so a shim would have nothing to translate. + +### Fixed + +- **The three frame constructors never type-checked `index`** (register C-93, Tier 2). They + validated `y_pred` four times — coerced it, checked dtype, ndim, row count — and read + exactly one attribute off `index`: `n_rows`. A frame has one. So + `PredictionFrame(values, another_frame)` constructed silently, as did any object exposing + `n_rows`. + + The half that made it Tier 2: every summarizer reads `.values` and `.n_rows` and none + reads `.index`, so a malformed frame returned **numerically correct** answers from + `collapse`, `map_estimate` and `hdi` — and **`assert_summarizer_contract` certified it**. + That checker is published under ADR-016 and consumers run it in their own CI, and + `docs/CICs/Conformance.md` names "a checker that cannot detect a violation" as the failure + mode it must never have. A false pass is worse than no checker, because it is evidence a + consumer is entitled to rely on. + + The checker keeps its own assertion even though construction now makes it unreachable by + ordinary means: consumers run the suite against their own frame factories, and + `with_metadata` already builds frames through `__new__` rather than `__init__`. + + Found by a falsification audit run against the claim that this package was finished. + +- **Same-level alignment leaked a private attribute** (register C-94). + `reindex`, `reindex_fill`, `is_superset_of` and `intersect` raised + `AttributeError: '...' object has no attribute '_level'` — naming a private attribute of a + class the caller never mentioned — where ADR-008 requires `ValueError`/`TypeError`. All + four route through one private helper, so one guard fixes the family. + +- **`map_estimate` crashed obscurely on non-finite draws** (register C-57). An `inf` draw + produced a `nan` bin index whose `astype(intp)` cast overflowed to the int-min sentinel, + giving `IndexError: index -9223372036854775808 is out of bounds`. It now raises + `ValueError` naming the cause, like `exceedance` and `expected_shortfall` have since + v1.5.0 and v1.6.0. + +### Changed + +- **`frame.values` is write-protected** (register C-66) — the enforce ADR-025 deferred, + riding this MAJOR exactly as planned. + + It did not ship as the one-liner ADR-025 recorded. `self._values.setflags(write=False)` + would have been a defect: `coerce_values` returns the caller's *own* array when it is + already `float32` — the zero-copy guarantee — so it silently makes **the caller's** array + read-only too. Measured, not reasoned about: the naive form does flip + `caller.flags.writeable` to `False`. The frames take a read-only **view** instead, which + locks the frame's buffer, leaves your array alone, and still shares memory. A `np.memmap` + keeps its subclass and its zero-copy. + +- **`CONFORMANCE_FLOOR` `1.0.0` → `2.0.0`.** The first move since the freeze, because + `assert_summarizer_contract` now rejects a frame that misreports its own index. Every + consumer's CI will assert a new contract version — that is what the constant is for, but + it has never happened before. + +### Changed — governance + +- **ADR-028** records the decision, the migration and the alternatives — including the three + that were rejected and why (deferring it as C-66 was deferred; fixing only the checker; + using a `Protocol` check, which fails because `SpatioTemporalIndexed` describes a *frame* + and `SpatioTemporalIndex` does not even satisfy it). +- **ADR-018 amended** — the freeze it declares has now been broken once, deliberately. +- **ADR-025 amended** — its "by convention" title is history, and the one-line fix it + recorded would have been wrong. +- **C-43 declined as a rider, in writing.** C-13's pre-tag checklist names it, so passing it + over silently would have been the failure this register has recorded three times. The two + binning functions are not two implementations of one concept: one is a deliberately + approximate clipped-linear bucket for a heuristic flag, the other reproduces + `numpy.histogram`'s edge-exact path bit-for-bit and is ulp-sensitive across numpy + versions. Its precondition is rewritten to `#89` alone, because "#89 or a MAJOR" became + false the moment this released. +- **New: register C-95** — `SpatioTemporalIndex` still write-protects the caller's + identifier arrays in place, the hazard C-66 had to avoid for the frames. Left alone + deliberately: that is ADR-025's own reasoning applied consistently rather than an + exception made because a MAJOR was already open. + ## [1.11.0] — 2026-08-18 **The published MAP-containment law was wrong on tied draws, and the governance documents diff --git a/CLAUDE.md b/CLAUDE.md index 137b4b8..964c3e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,15 +6,20 @@ at the **root of the platform dependency DAG**, plus two sibling operation packa in the same wheel. numpy only; depends on nothing internal; every other repo depends *toward* it. -> **Status:** **released — v1.11.x on PyPI**, public API **frozen since v1.0.0** -> (ADR-018; everything after is additive, `CONFORMANCE_FLOOR` stays `1.0.0`). +> **Status:** **released — v2.x on PyPI**. The v1 API was frozen from v1.0.0 (ADR-018) +> and **broken once, deliberately, in 2.0.0** (ADR-028): a frame's `index` must now +> actually be a `SpatioTemporalIndex`, and `frame.values` is write-protected. +> `CONFORMANCE_FLOOR` moved `1.0.0` → `2.0.0` — its first move. Everything else is +> additive, and the bar for a further MAJOR stays deliberately high. > Consumers install `views-frames` and validate against the published conformance > suite (`views_frames.conformance`, ADR-016). See `CHANGELOG.md` for the release > history and `README.md` §status for the version chronicle. ## Maintenance mode -**This package is finished.** It is released, its public API has been frozen since v1.0.0, +**This package is finished.** It is released, its public API was frozen at v1.0.0 and has been +broken exactly once since (2.0.0, ADR-028 — a validation hole found by attacking the claim +that it was finished), and its governance documents have been checked against the code and are now asserted by CI. Work here should be rare, small, and caused by something outside this repository. @@ -102,8 +107,9 @@ CI additionally gates 100% line+branch coverage *only* under `io/`. Enforced by `tests/test_import_enforcement.py` (ADR-002). 2. **Immutable value objects.** Operations return new frames; structural ops share the buffer (zero-copy); only reductions allocate (register C-07). Enforced for the - *index*; **by convention** for the value buffer (writeable on purpose to preserve - zero-copy/mmap — mutating `.values` in place is unsupported; ADR-025, C-66 rider). + *index* and, since 2.0.0, for the value buffer too — a read-only **view**, so + zero-copy and mmap survive and the caller's own array stays writeable (ADR-028, + register C-66; convention-only until then, ADR-025). 3. **Fail loud.** Invariants raise `ValueError`/`TypeError` at construction and at every validation guard; the guarantee is *structural*, not temporal (`time` is opaque; register C-11). diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 227bd7a..ba3cabd 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -39,7 +39,10 @@ for name, mod in (('views_frames', c), ('views_frames_summarize', s), ('views_fr " ``` -- **Conformance-floor version:** `1.0.0` (`views_frames.conformance.CONFORMANCE_FLOOR`). +- **Conformance-floor version:** `2.0.0` (`views_frames.conformance.CONFORMANCE_FLOOR`). + Moved from `1.0.0` on 2026-08-18 with the 2.0.0 MAJOR (ADR-028) — the first move since + the freeze, because `assert_summarizer_contract` now rejects a frame that misreports + its own index. - The floor is a **single governed version every consumer runs regardless of its runtime pin** — this is what makes the suite test "all consumers agree," not "my adapter vs my pin" (closes register C-10). The floor is bumped deliberately, diff --git a/README.md b/README.md index 94be60f..229a005 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ > containers (`FeatureFrame`, `PredictionFrame`, and their anticipated siblings) > that every other repo depends on and that depends on nothing internal. > -> **Status:** **v1.11.0 — frozen API, published to PyPI** (frozen since v1.0.0, ADR-018; the +> **Status:** **v2.0.0 — published to PyPI** (frozen at v1.0.0, ADR-018, and broken once in 2.0.0, ADR-028; the > v1.1 surface is > purely additive — the coherent posterior summary, ADR-019; v1.2.0 rebuilt the tower > `outside-in`, C-44; v1.3.0 makes the tower summary distribution-agnostic — no magnitude @@ -21,7 +21,7 @@ > `research/figures/` tower-figure toolkit; v1.10.0 adds the **dense-grid fill** > primitive — `reindex_fill(other, *, fill_value)` on all three frames + > `SpatioTemporalIndex.cartesian` + the published `assert_reindex_fill_law`, ADR-026, -> unblocking pandas-free FAO ingestion; v1.10.1 makes `io.arrow.load` fail loudly on a parquet whose row order breaks the wire contract, #199; v1.10.2 changes no behaviour — it arms the documentation check in CI, turns eleven falsification tests from README-wording checks into behaviour checks, and records ADR-027 declining the construction shortcut; v1.11.0 corrects the published MAP-containment law, which was wrong on tied draws and so failed ~6% of rows on integer count posteriors — register C-88 — and makes the two IO codecs agree on a non-JSON metadata value, C-90; the rest of that release makes the governance documents match the code and adds the CI checks that keep them matching). This +> unblocking pandas-free FAO ingestion; v1.10.1 makes `io.arrow.load` fail loudly on a parquet whose row order breaks the wire contract, #199; v1.10.2 changes no behaviour — it arms the documentation check in CI, turns eleven falsification tests from README-wording checks into behaviour checks, and records ADR-027 declining the construction shortcut; v1.11.0 corrects the published MAP-containment law, which was wrong on tied draws and so failed ~6% of rows on integer count posteriors — register C-88 — and makes the two IO codecs agree on a non-JSON metadata value, C-90; the rest of that release makes the governance documents match the code and adds the CI checks that keep them matching; **v2.0.0 is the first MAJOR** — a falsification audit found that a frame's `index` was never type-checked, so a frame (or any object with an `n_rows`) was accepted silently and the published `assert_summarizer_contract` then certified the result, and it now raises `TypeError`; riding it, `frame.values` becomes read-only (C-66) and `map_estimate` rejects non-finite draws (C-57), and `CONFORMANCE_FLOOR` moves to `2.0.0` — a correct consumer changes its version constraint and re-locks, nothing else, ADR-028). This > README is the design > bible; the contract it specifies is realised in `src/views_frames/` (index, frames, > io, conformance suite) plus the `src/views_frames_summarize/` sibling package @@ -233,13 +233,14 @@ makes it safe to depend on from everywhere (SDP). 3. **Immutable value objects.** A frame is validated at construction and then treated as read-only. Operations (`collapse`, `select`, `with_metadata`) **return new frames**; they never mutate in place. (Directly forbids the - C-184 cross-repo-mutation anti-pattern.) **Enforced for the index, by - convention for the value buffer:** the identifier arrays (`time`/`unit`) are - write-protected (`setflags(write=False)`); the `values` buffer is left - **writeable on purpose** — so structural ops can share it zero-copy — and is - immutable *by convention*: mutating `.values` in place is unsupported and may - silently corrupt buffer-sharing frames (ADR-025 / register C-63; enforcing it - is a deferred MAJOR-rider). **Copy-vs-view:** structural and + C-184 cross-repo-mutation anti-pattern.) **Enforced for the index, and since 2.0.0 + for the value buffer too:** the identifier arrays (`time`/`unit`) are + write-protected (`setflags(write=False)`); the `values` buffer was left + **writeable on purpose** — so structural ops can share it zero-copy — and was + immutable *by convention* **until 2.0.0**, which enforced it: `values` is now a + read-only **view**, so an in-place mutation raises instead of silently corrupting + buffer-sharing frames, while zero-copy, mmap and the caller's own array are all + preserved (ADR-028 / register C-66; the convention-only era is ADR-025 / C-63). **Copy-vs-view:** structural and metadata-only operations (`with_metadata`, contiguous `select`) return frames that **share** the underlying `values` buffer (numpy view / zero-copy), and a `mmap`-backed frame stays `mmap`-backed — a new frame must never copy a diff --git a/docs/ADRs/018_api_freeze_v1.md b/docs/ADRs/018_api_freeze_v1.md index 0066368..657e938 100644 --- a/docs/ADRs/018_api_freeze_v1.md +++ b/docs/ADRs/018_api_freeze_v1.md @@ -1,11 +1,31 @@ # ADR-018: API freeze at v1.0.0 -**Status:** Accepted +**Status:** Accepted (amended 2026-08-18 — see *Amendment* below) **Date:** 2026-06-21 **Deciders:** VIEWS platform maintainers (keystone owner) **Consulted:** two rounds of consumer review **Informed:** all `views_frames` consumers +> **Amendment (2026-08-18, ADR-028 / register C-13).** **The freeze has been broken once, +> deliberately, in 2.0.0** — the first MAJOR since this ADR was written. +> +> What changed: the three frames' constructors now reject an `index` argument that is not a +> `SpatioTemporalIndex`. That is a **tightening of an invariant** on the constructor shapes +> frozen below, and therefore a MAJOR under `GOVERNANCE.md`. Before 2.0.0 the constructors +> read one attribute off `index` — `n_rows` — so a frame, or any object with an `n_rows`, +> was accepted; the published `assert_summarizer_contract` then certified the result. +> +> Shipping alongside it, as riders on the same bump: `frame.values` is now write-protected +> (register C-66, the enforce ADR-025 deferred), the same-level alignment ops raise +> `TypeError` rather than leaking a private attribute, and `map_estimate` rejects non-finite +> draws (register C-57). `CONFORMANCE_FLOOR` moved `1.0.0` → `2.0.0`, its first move. +> +> **The frozen list below is otherwise unchanged** — no name was removed, renamed or +> re-signatured, and no consumer passing a real `SpatioTemporalIndex` is affected. What +> narrowed is the set of inputs that were never valid. ADR-028 carries the decision, the +> full migration table and the reasoning; read it before treating this as licence for a +> second MAJOR. + --- ## Context diff --git a/docs/ADRs/025_value_buffer_immutability_by_convention.md b/docs/ADRs/025_value_buffer_immutability_by_convention.md index a31f065..70620b9 100644 --- a/docs/ADRs/025_value_buffer_immutability_by_convention.md +++ b/docs/ADRs/025_value_buffer_immutability_by_convention.md @@ -1,12 +1,33 @@ # ADR-025: Value-buffer immutability is by convention; only the index is enforced -**Status:** Accepted +**Status:** Accepted — superseded in part by ADR-028 (2026-08-18); see *Amendment* **Date:** 2026-06-28 **Deciders:** VIEWS platform maintainers **Consulted:** repo-assimilation + test-review (2026-06-27, the audit that surfaced C-63) **Informed:** views-pipeline-core, views-datafactory (consumers that hold frames) +> **Amendment (2026-08-18, ADR-028 / register C-66).** **The deferred enforce has shipped.** +> `frame.values` is write-protected as of 2.0.0, so the "by convention" half of this ADR's +> title now describes history rather than current behaviour. The reasoning below for *why* +> it was deferred — that tightening frozen surface is a MAJOR disproportionate to a hole no +> code in the ecosystem hits — was correct and is why it waited; 2.0.0 happened for another +> reason and carried it for free, exactly as C-66 planned. +> +> **It did not ship as the one-liner recorded below.** This ADR names the fix as +> `self._values.setflags(write=False)`. That would have been wrong: `coerce_values` returns +> the caller's own array when it is already `float32` — the C-07 zero-copy guarantee — so +> the one-liner silently makes the **caller's** array read-only too. Measured, not assumed: +> the naive form flips `caller.flags.writeable` to `False`. The frames take a **read-only +> view** instead, which locks the frame's buffer, leaves the caller's array writeable, and +> still shares memory; the memmap path keeps its subclass and zero-copy through `.view()`. +> +> One inconsistency is left standing on purpose: `SpatioTemporalIndex` still write-protects +> the identifier arrays in place, so it *does* reach back into the caller's arrays, and its +> comment called them "views" when `ascontiguousarray` returns the same object for +> already-contiguous input. The comment now says what the code does; the behaviour is +> long-standing and out of 2.0.0's scope. + --- ## Context diff --git a/docs/ADRs/028_major_2_0_0_index_type_enforcement.md b/docs/ADRs/028_major_2_0_0_index_type_enforcement.md new file mode 100644 index 0000000..f2c4433 --- /dev/null +++ b/docs/ADRs/028_major_2_0_0_index_type_enforcement.md @@ -0,0 +1,221 @@ +# ADR-028: 2.0.0 — a frame's index must actually be an index + +**Status:** Accepted +**Date:** 2026-08-18 +**Deciders:** VIEWS platform maintainers +**Consulted:** the falsification audit of 2026-08-18; register C-13's pre-tag checklist +**Informed:** views-faoapi, views-postprocessing, views-crafdapi (the three pinned consumers) + +--- + +## In short + +Until now you could hand a frame **anything** as its index and it would be accepted. A +different frame. A bare object with an `n_rows` attribute. The frame was built, it looked +fine, and the published conformance suite said it was fine. + +From 2.0.0 that raises `TypeError`. + +**Nothing a correct consumer does changes.** If you pass a `SpatioTemporalIndex` — which is +what the type hint has always said, and what every example and every test does — you will +not notice this release. What changes is that the wrong thing now fails immediately instead +of much later and somewhere else. + +This is the package's **first MAJOR** and the first move of `CONFORMANCE_FLOOR` since the +v1.0.0 freeze. + +--- + +## What was wrong + +Construction validated `y_pred` four times over — coerced it, checked its dtype, its +dimensionality, its row count. It never checked `index` at all. It read exactly one +attribute off it, `n_rows`, to compare against the value array's first axis. + +A frame has an `n_rows`. So this worked: + +```python +other = PredictionFrame(values, index) +frame = PredictionFrame(values, other) # `other` is a FRAME, not an index +``` + +So did this: + +```python +class NotAnIndex: + n_rows = 2 + +PredictionFrame(values, NotAnIndex()) # also fine, apparently +``` + +The resulting object was not a frame in any meaningful sense — `frame.index.time` raised +`AttributeError` — but it was *constructed*, and it would travel. Every summarizer reads +`.values` and never `.index`, so `collapse`, `map_estimate`, `hdi` and the rest returned +**numerically correct answers** on it. The failure surfaced only when something finally +touched the index, far from the line that caused it. + +**The part that made this worth a MAJOR:** `assert_summarizer_contract` — published under +ADR-016, run by consumers in *their* CI — returned cleanly for such a frame. A checker +whose stated most important job, in `docs/CICs/Conformance.md`, is to **fail** when handed +a frame that misreports itself, was issuing a false pass. + +Two smaller defects from the same audit ship with it: the same-level alignment ops leaked +`AttributeError: '...' object has no attribute '_level'` — a private attribute of a class +the caller never named — where ADR-008 requires `ValueError`/`TypeError`; and `map_estimate` +crashed with a bare `IndexError` on a non-finite draw where its two sibling estimators raise +a clean `ValueError`. + +--- + +## Why this is a MAJOR and not a quiet fix + +`GOVERNANCE.md` lists **"tightening an invariant"** as a MAJOR bump, with no exception for +guards that are obviously right. ADR-018 freezes "the three frames, **their constructor +shapes**". A call that returns a valid object today raises tomorrow. + +The precedent is **ADR-025 / register C-66**, and it is exact. That ADR considered +`self._values.setflags(write=False)` — one line, mandated in spirit by ADR-008, with an +audit confirming nothing in the ecosystem exercised the gap — and still classified it MAJOR +and deferred it, because tightening frozen surface is MAJOR *regardless of how safe the +guard is*. + +The test this repo actually applies is **"does any currently-succeeding call start +raising?"** C-57's `map_estimate` hardening answers no — the input already crashed, just +uglily — so it is additive and rides freely. The index guard answers yes. So: MAJOR. + +The bar for a MAJOR is deliberately high here, and it is being met by a *validation* fix +rather than a feature. That is the honest reading of our own rule, and the alternative — +leaving a published checker issuing false passes indefinitely because the fix is +inconveniently classified — is worse. + +--- + +## Decision + +1. The three frame constructors raise `TypeError` unless `index` is a `SpatioTemporalIndex`. +2. `SpatioTemporalIndex._require_same_level` raises `TypeError` on a non-index argument, + which covers `searchsorted`/`reindex`, `is_superset_of` and `intersect` in one place. +3. `assert_summarizer_contract` asserts its frame's index is a `SpatioTemporalIndex`. +4. `CONFORMANCE_FLOOR` moves `1.0.0` → `2.0.0`. +5. The MAJOR carries its riders (register C-13): **C-66** value-buffer write protection and + **C-57** `map_estimate`'s non-finite guard. **C-43** is declined — see below. + +### Riders, and the one declined + +**C-66 did not ship as written.** ADR-025 records the fix as +`self._values.setflags(write=False)`. That would have been a mistake: `coerce_values` +returns the caller's *own* array when it is already `float32`, which is the C-07 zero-copy +guarantee, so the one-liner silently makes the **caller's** array read-only — action at a +distance well outside this contract. Measured rather than assumed: the naive form does flip +`caller.flags.writeable` to `False`. The frames take a **read-only view** instead, which +locks the frame's buffer, leaves the caller's array alone, and still shares memory. The +memmap path keeps its subclass and its zero-copy through `.view()`. + +**C-43 is declined, deliberately.** C-13's checklist names it a rider, so passing it over +silently would be the "claimed enforcement that does not exist" failure this repo has +recorded three times. `bimodality._coarse_counts` is a clipped linear bucket, deliberately +approximate, for a heuristic flag. `point._batched_map` reproduces `numpy.histogram`'s +edge-exact path bit-for-bit and is ~1-ulp sensitive across numpy versions (the C-24 +portability saga). They are not two implementations of one concept — they are two different +algorithms that both happen to bin. Extracting a shared helper forces one onto the other's +path and changes the output of either `bimodality` or the frozen `map_estimate`, to serve a +Tier-4 entry the register itself says has no correctness or reliability impact. C-43's +precondition is rewritten to `#89` alone. + +### Migration + +**For nearly every consumer: change your version constraint and re-lock. Nothing else.** + +```diff +- views-frames = ">=1.10.2,<2" ++ views-frames = ">=2.0.0,<3" +``` + +Your code breaks only if it was already broken: + +| If you… | Before | Now | +|---|---|---| +| pass a `SpatioTemporalIndex` as `index` | works | works, unchanged | +| pass a frame or other object as `index` | silently constructed | `TypeError` at construction | +| pass a non-index to `reindex`/`reindex_fill` | `AttributeError` on `_level` | `TypeError`, naming what was expected | +| mutate `frame.values` in place | silently corrupted every frame sharing the buffer | `ValueError: assignment destination is read-only` | +| call `map_estimate` on `inf`/`NaN` draws | bare `IndexError` | `ValueError` naming the cause | + +The in-place `.values` mutation is the only row where working code could plausibly stop +working — and ADR-025 documented that operation as unsupported, so code doing it was +relying on something the contract already denied. + +**No `from_legacy_*` shim.** GOVERNANCE's MAJOR process asks for one "where a consumer +format changes". No wire format, no serialized layout and no signature changes here; only +the set of inputs that were never valid narrows. A shim would have nothing to translate. + +--- + +## Alternatives we considered + +**Defer to some later MAJOR, as C-66 was deferred.** The C-66 pattern — write the guard and +the red test, register the entry, let it ride — was the recommendation until the maintainer +chose otherwise. It was defensible: no consumer had hit the gap. It was rejected because a +MAJOR that never arrives leaves a published checker issuing false passes for an unbounded +time, and because deferring meant carrying C-66 and C-57 unshipped as well. Deferring is +cheap exactly once; this would have been the third deferral onto the same imaginary release. + +**Fix only the checker, leave construction alone.** Tempting, since the checker is the part +that lies, and it would not have been a MAJOR. Rejected: it treats the symptom. The +constructor is where the invariant belongs (ADR-008: validation happens in `__init__`, +before the object is usable), and a malformed frame would still exist and still travel — it +would merely be caught by one checker rather than never. + +**Make the guard a `Protocol` check rather than `isinstance`.** ADR-009 has consumers depend +on protocols, so this looked more in keeping. Rejected on the facts: +`SpatioTemporalIndexed` describes a **frame** (`n_rows`, `identifiers`, `index`), not an +index — `SpatioTemporalIndex` does not even satisfy it, having no `.index`. And +`runtime_checkable` protocols check member *presence* only, which is precisely the weak +duck-type that caused this. The constructors' own annotation has always said +`SpatioTemporalIndex`; the guard now matches it. + +**Extract the guard into `_validation.py`.** It is where the other construction guards live. +Rejected on a hard constraint: `index.py` imports `_validation.py`, so `_validation.py` +cannot import `SpatioTemporalIndex` back without a cycle — and `tests/test_import_enforcement.py` +plus the `import-linter` contracts exist to keep that graph acyclic. Written three times +instead, which ADR-011 Option C already accepts as the price of having no shared base. + +--- + +## Consequences + +**Good.** A frame cannot be built with a non-index. A published checker no longer certifies +frames it was written to reject. The alignment family names what it expected instead of +leaking a private attribute. `frame.values` is immutable in fact rather than by convention, +closing the Tier-2 mechanism behind C-63. `map_estimate` fails like its siblings. + +**Costly.** Three consumer repos are pinned `<2` and reach this only by re-locking, and each +needs an adoption issue before tagging per C-13. `CONFORMANCE_FLOOR` moving means every +consumer's CI asserts a new contract version — deliberate, and the reason the constant +exists, but it is the first time it has happened. + +**Unproven.** That no consumer relies on the old leniency. The audit could measure this +repo and read the three consumers' pins; it could not run their suites. The migration table +above is the mitigation — every behaviour that changes is enumerated, so a consumer can +check their own code against it without reading this ADR's reasoning. + +**A precedent set.** This is the first MAJOR and the first floor move. It should not become +routine: GOVERNANCE's closing note says that a package needing frequent MAJOR bumps is not +abstract enough. The correct reading of this release is that a validation hole present since +v1.0.0 was found by deliberately attacking the claim that the package was finished — not +that the freeze is negotiable. + +--- + +## References + +- Register **C-13** (the pre-tag checklist this release executes), **C-66** (shipped here), + **C-57** (shipped here), **C-43** (declined here), **C-63** (the Tier-2 mechanism C-66 + closes), **C-07** (the zero-copy guarantee the read-only view preserves). +- **ADR-025** — the deferral this release spends; its reasoning is the direct precedent for + classifying the index guard as MAJOR. +- **ADR-018** — the frozen surface, amended by this release. +- **ADR-008** — validation happens in `__init__`, with `ValueError`/`TypeError`. +- **ADR-011** — Option C, which makes the three-times guard the house pattern. +- **ADR-016** — why a false pass in the published suite is a cross-repo defect. +- `GOVERNANCE.md` — "tightening an invariant" = MAJOR; the cross-repo bump process. diff --git a/docs/ADRs/README.md b/docs/ADRs/README.md index 59bd425..adf1633 100644 --- a/docs/ADRs/README.md +++ b/docs/ADRs/README.md @@ -67,15 +67,16 @@ it resolves. - **ADR-015** — [`SpatialLevel` relocation](015_spatiallevel_relocation.md). Identifier vocabulary only; fix the C-65 time-first index-tuple and the `priogrid_gid`/`priogrid_id` inconsistency, not port them. Resolves C-18. - **ADR-016** — [Conformance-floor + ownership/release](016_conformance_floor_and_ownership.md). Governed conformance-floor every consumer runs in CI; named owner + cross-repo MAJOR-bump process. Resolves C-05, C-10, C-13; carries deferred ADR-004's scope. - **ADR-017** — [Posterior/sample summarization is a sibling package](017_summarization_is_a_sibling_package.md). Sample-axis reduction (MAP/HDI/quantiles/collapse) lives in the `views_frames_summarize` sibling package, not the leaf; the leaf keeps only the structural `sample_count`/`is_sample`. -- **ADR-018** — [API freeze at v1.0.0](018_api_freeze_v1.md). v1.0.0 freezes the public surface (frames + index + protocols + conformance + summarizer estimators); ends the pre-1.0 breaking-in-MINOR latitude; records the frozen `(time, unit)` cross-level key, the row-uniqueness stance, and `select`/`reindex`. Resolves C-27 (with GOVERNANCE). +- **ADR-018** — [API freeze at v1.0.0](018_api_freeze_v1.md). v1.0.0 freezes the public surface (frames + index + protocols + conformance + summarizer estimators); ends the pre-1.0 breaking-in-MINOR latitude; records the frozen `(time, unit)` cross-level key, the row-uniqueness stance, and `select`/`reindex`. Resolves C-27 (with GOVERNANCE). **Amended 2026-08-18 (ADR-028):** the freeze was broken once, deliberately, in the 2.0.0 MAJOR — the constructors now reject a non-index `index`. No name was removed or re-signatured; the set of never-valid inputs narrowed. - **ADR-019** — [Coherent posterior summary — the constrained-nested HDI tower](019_coherent_posterior_summary_hdi_tower.md). The `views_frames_summarize` tower estimators (`hdi_tower`/`tower_point`/`bimodality`/`summarize_tower`): nested-by-construction HDIs on a fixed canonical grid, the mass-aware `tip_mass` MAP, and a fail-loud config. Amended for the outside-in rebuild (C-44) and the distribution-agnostic quiet rule (C-45). - **ADR-020** — [The `MetricFrame` contract lives in views-evaluation, on the views-frames substrate](020_metricframe_contract_home.md). Ratifies (under the GH#109 driver) that evaluation-output types stay out of the leaf; the leaf provides only the `FrameMetadata` substrate and the conformance/IO patterns. Records the provenance split (C-47) and the envelope-drift mitigation (C-46); keeps `CONFORMANCE_FLOOR` at 1.0.0. Extends C-01. - **ADR-021** — [Threshold exceedance-probability estimator (`exceedance`)](021_exceedance_probability_estimator.md). Adds the `views_frames_summarize` per-row survival-fraction estimators (`exceedance`/`exceedance_reducer`, `P(Y > c)`): caller-supplied required thresholds (no default, no config), strict `>`, fail-loud NaN, geography-blind (country = `aggregate_distributions` → `exceedance`). Settles D-07/D-08; records C-49/C-50; additive/MINOR, `CONFORMANCE_FLOOR` stays 1.0.0. - **ADR-022** — [Worst-case scenario summary — the `expected_shortfall` tail mean](022_worst_case_expected_shortfall.md). Adds the `views_frames_summarize` worst-case estimator `expected_shortfall` (mean of the worst `⌈t·S⌉` draws — a coherent tail-risk measure, the companion to `exceedance`): caller-supplied required tail levels (no default, no config), upper-tail only, **never `max`**, fail-loud NaN, geography-blind. Best-case ships no code (low quantile + `exceedance(0)`). Settles D-10; records C-55/C-56; additive/MINOR (v1.6.0), `CONFORMANCE_FLOOR` stays 1.0.0. - **ADR-023** — [Forecast reconciliation is a sibling package (`views_frames_reconcile`)](023_reconciliation_is_a_sibling_package.md). Adds a third sibling package (mirrors ADR-017): the numpy-only, `views_frames`-only forecast reconciler (`reconcile_proportional`/`ReconciliationModule` — make pgm grid forecasts sum to cm country totals per draw), relocated **WET** from views-postprocessing and proven bit-identical. Charter: frame-reconciliation algorithms only; **never fetch the mapping** (injected as arrays, like `cross_level_align`); no IO/scoring/plotting/foreign `views_*`. Import-DAG `views_frames_reconcile → {views_frames}`; additive/MINOR (v1.7.0), `CONFORMANCE_FLOOR` stays 1.0.0. **Amended 2026-06-27 (#143, v1.8.0):** the sample-count contract — a point cm (`sample_count == 1`) is broadcast across the grid's draws; the equal-count path stays bit-exact. - **ADR-024** — [Principled joint probabilistic reconciliation — design direction & deferral](024_principled_joint_reconciliation_design.md). Design-only (no code): per-draw `proportional` is an information-losing approximation that pairs grid-draw `s` with country-draw `s` across **independently-trained** models with no shared draw identity. Names the **draw-identity contract** principled reconciliation requires (shared ensemble / copula coupling), fixes the upgrade as a **future sibling module** (probabilistic reconciliation / MinT — never a change to `proportional`), and **defers** implementation until a consumer needs calibrated joint tails *and* the country model can supply the identity/coupling. Records C-62; corrects the ambiguous `proportional.py` "C-37" reference (views-frames C-37 is unrelated/resolved; the lineage is views-postprocessing C-37). -- **ADR-025** — [Value-buffer immutability is by convention; only the index is enforced](025_value_buffer_immutability_by_convention.md). Corrects the "immutable value objects" contract to match the code: only the index (`time`/`unit`) is `setflags(write=False)`-enforced; the frame **value buffer is immutable by convention** (mutating `.values` in place is unsupported and may corrupt buffer-sharing frames), left writeable to preserve zero-copy / `mmap` (C-07). The `setflags`-enforce on `.values` would be "tightening an invariant" on a frozen-surface member (a MAJOR, GOVERNANCE/ADR-018) for a hole nothing exercises, so it is recorded as a **deferred MAJOR-rider** (added free on the next MAJOR), not done now. Resolves C-63; docs-only, `CONFORMANCE_FLOOR` stays 1.0.0. +- **ADR-025** — [Value-buffer immutability is by convention; only the index is enforced](025_value_buffer_immutability_by_convention.md). Corrects the "immutable value objects" contract to match the code: only the index (`time`/`unit`) is `setflags(write=False)`-enforced; the frame **value buffer is immutable by convention** (mutating `.values` in place is unsupported and may corrupt buffer-sharing frames), left writeable to preserve zero-copy / `mmap` (C-07). The `setflags`-enforce on `.values` would be "tightening an invariant" on a frozen-surface member (a MAJOR, GOVERNANCE/ADR-018) for a hole nothing exercises, so it was recorded as a **deferred MAJOR-rider**. **Amended 2026-08-18 (ADR-028): the enforce shipped** in 2.0.0, riding that MAJOR exactly as planned — though as a read-only *view*, not the `setflags` one-liner this ADR recorded, which would have made the caller's own array read-only via the C-07 zero-copy return. Resolves C-63 and C-66. - **ADR-026** — [Dense-grid fill is a leaf primitive](026_dense_grid_fill_primitive.md). Adds `frame.reindex_fill(other, *, fill_value)` (align with **no** superset requirement; present rows bit-exact, absent rows filled; `fill_value` keyword-only + required) on all three siblings, `SpatioTemporalIndex.cartesian(times, units, level)` (dense product index, time-major, explicit arrays only, fails loud on duplicated inputs), and the published `assert_reindex_fill_law`. Closes #203 (faoapi's "views-frames has no fill primitive", the last pandas blocker on FAO ingestion, faoapi #242); rejects a derivation rule and a `reindex` mode-kwarg (D-09/C-52 precedents). Additive MINOR (v1.10.0); `CONFORMANCE_FLOOR` stays 1.0.0. +- **ADR-028** — [2.0.0 — a frame's index must actually be an index](028_major_2_0_0_index_type_enforcement.md). **The first MAJOR, and the first move of `CONFORMANCE_FLOOR`** (`1.0.0` → `2.0.0`). Until 2.0.0 the constructors read one attribute off `index` — `n_rows` — so a frame, or any object with an `n_rows`, was accepted silently, and the published `assert_summarizer_contract` then **certified** the result: a checker issuing a false pass, the failure mode `docs/CICs/Conformance.md` says it must never have. Found by a falsification audit of the claim that this package was finished. Carries its riders per C-13: **C-66**'s value-buffer write-protection (as a read-only view) and **C-57**'s `map_estimate` non-finite guard; **declines C-43** with its reason recorded. Also fixes the alignment ops' `AttributeError` leak of a private attribute. A correct consumer changes its version constraint and re-locks; nothing else. Resolves C-66, C-57 and the audit's findings; amends ADR-018 and ADR-025. - **ADR-027** — [Frame construction stays two-step; the convenience factory is declined](027_decline_construction_convenience_factory.md). **Declines #113**, which asked for a one-line shortcut for building a `PredictionFrame`. Nothing changes: callers keep building the `SpatioTemporalIndex` and then the frame. The request was never built and never blocked anyone — the engine repos migrated using the two-step form — and because the surface is frozen (ADR-018), adding a shortcut would be permanent. Stating which rows your numbers describe is what this package should make callers say out loud (ADR-003), not hide. The ADR records the design already agreed for any future need, and what would justify revisiting it. Resolves C-52/C-53/C-54; docs-only, `CONFORMANCE_FLOOR` stays 1.0.0. --- diff --git a/docs/CICs/Conformance.md b/docs/CICs/Conformance.md index 2f66f97..3fe9abe 100644 --- a/docs/CICs/Conformance.md +++ b/docs/CICs/Conformance.md @@ -73,9 +73,11 @@ target units. A mapping keyed by unit alone would pass every other check here an be wrong, because borders move. **`CONFORMANCE_FLOOR`** — the version string this suite belongs to. It moves only when the -contract itself changes, **not** when the package version does. It has stayed `"1.0.0"` -across every release since the freeze (ADR-018), which is the intended behaviour, not an -oversight. +contract itself changes, **not** when the package version does. It stayed `"1.0.0"` across +every release from the freeze (ADR-018) until **2026-08-18**, when 2.0.0 moved it to +`"2.0.0"` — the first move, and the proof the constant is governed rather than decorative. +It moved because `assert_summarizer_contract` began rejecting a frame whose index is not a +`SpatioTemporalIndex`, which narrows what the suite accepts (ADR-028). **Assertions must be enabled.** Every entry point calls `_require_assertions()` first and raises `RuntimeError` under `python -O`. Without it the entire suite would silently pass diff --git a/docs/CICs/FeatureFrame.md b/docs/CICs/FeatureFrame.md index 67ada12..a52b297 100644 --- a/docs/CICs/FeatureFrame.md +++ b/docs/CICs/FeatureFrame.md @@ -40,8 +40,9 @@ array `y_features (N, F, S)` float32 aligned to a `SpatioTemporalIndex`, carryin frame from a 2-D `(N, F)` array of unsampled features, adding the trailing sample axis to give `(N, F, 1)`. It is ordinary, supported surface — not a deprecated shim. - Immutable with copy-vs-view semantics identical to `PredictionFrame` (C-07): the - **index is enforced** read-only; the **value buffer is immutable by convention** - (writeable for zero-copy — in-place `.values` mutation is unsupported; ADR-025 / C-63). + **index is enforced** read-only, and **since 2.0.0 so is the value buffer** — a + read-only *view*, preserving zero-copy and `mmap` while leaving the caller's array + writeable (ADR-028 / C-66; convention-only until then, ADR-025 / C-63). - Row ops return new frames preserving `feature_names`: `select(positions | mask)` and `reindex(other)` — the latter raises unless this index is a superset of `other`; selection **copies** the selected `values`. @@ -85,6 +86,12 @@ Violations raise at construction (ADR-008). - Raises on non-`float32`/object dtype, shape/length mismatch (incl. `len(feature_names) != F`), or incomplete identifiers. Structural, not temporal (register C-11). +- Raises `TypeError` when `index` is not a `SpatioTemporalIndex` (since 2.0.0, ADR-028). + Before that, construction read one attribute off `index` — `n_rows` — so a frame, or any + object exposing `n_rows`, was accepted silently. +- Raises `TypeError` from `reindex`/`reindex_fill` on a non-index argument; it previously + leaked `AttributeError` naming a private attribute (ADR-008). +- Raises `ValueError` on in-place `.values` assignment — the buffer is write-protected. --- @@ -117,9 +124,12 @@ FeatureFrame(y_features=x2d, index=idx, feature_names=names) # raises # WRONG: calling a grid constructor on the frame — that adapter lives in datafactory FeatureFrame.from_grid(cube) # no such method here -# WRONG: mutating the value buffer in place — immutable *by convention*, not -# write-protected, so it does NOT raise; build a new frame (ADR-025 / register C-63). -ff.values[:] = 0 # unsupported: silent shared-buffer corruption, no error +# WRONG: mutating the value buffer in place — write-protected since 2.0.0, so this now +# raises rather than silently corrupting buffer-sharing frames (ADR-028 / register C-66). +ff.values[:] = 0 # ValueError: assignment destination is read-only + +# WRONG: passing something that is not an index — raises since 2.0.0 (ADR-028) +FeatureFrame(y_features=x, index=some_other_frame, feature_names=names) # TypeError ``` --- @@ -129,7 +139,9 @@ ff.values[:] = 0 # unsupported: silent shared-buffer corruption, no err - **Green:** construction validation incl. `feature_names` length; `select`/`reindex` parity (`test_frame_parity.py`); save/load round-trip preserving `feature_names`/`metadata`. - **Beige:** copy-vs-view property (`with_metadata` allocates no second buffer). -- **Red:** 2-D input / object dtype / mismatched `feature_names` raises; no-pandas +- **Red:** 2-D input / object dtype / mismatched `feature_names` raises, and a + non-`SpatioTemporalIndex` `index` raises — `tests/test_construction_red.py`; the value + buffer is read-only — `tests/test_properties.py`; no-pandas import-enforcement. --- diff --git a/docs/CICs/PredictionFrame.md b/docs/CICs/PredictionFrame.md index 94e4596..725ef83 100644 --- a/docs/CICs/PredictionFrame.md +++ b/docs/CICs/PredictionFrame.md @@ -37,9 +37,10 @@ ensemble samples `y_pred (N, S)` float32 aligned to a `SpatioTemporalIndex`. and always explicit (`S >= 1`; ADR-012). - Immutable: `with_metadata` returns a **new** frame sharing the `values` buffer (zero-copy); `mmap` propagates (register C-07). **Immutability is enforced for the - index** (`time`/`unit` are `setflags(write=False)`) and held **by convention for the - value buffer** (left writeable to preserve zero-copy — mutating `.values` in place is - unsupported and may corrupt buffer-sharing frames; ADR-025 / register C-63). + index** (`time`/`unit` are `setflags(write=False)`) and, **since 2.0.0, for the value + buffer as well** — held as a read-only *view*, so zero-copy and `mmap` survive and the + caller's own array is left writeable (ADR-028 / register C-66). It was immutable only + by convention until then (ADR-025 / register C-63). - Row ops return new frames: `select(positions | mask)` and `reindex(other)` — the latter raises unless this frame's index is a superset of `other`. Selection **copies** the selected `values` (only structural/metadata ops share the buffer). @@ -84,6 +85,10 @@ Violations raise at construction (ADR-008) — never log-and-continue. to float32 by copy at construction — accepted, not rejected (the no-copy fast path is float32-only, register C-07). The structural guarantee is **not temporal** (register C-11). +- Raises `TypeError` when `index` is not a `SpatioTemporalIndex` (since 2.0.0, ADR-028). Before that, construction read one attribute off `index` — `n_rows` — so a frame, or any object exposing `n_rows`, was accepted silently and the published summarizer checker then certified the result. +- Raises `TypeError` from `reindex`/`reindex_fill` when handed something that is not a `SpatioTemporalIndex`; it previously leaked `AttributeError` naming a private attribute (ADR-008 requires `ValueError`/`TypeError` at every guard). +- Raises `ValueError` on in-place `.values` assignment — the buffer is write-protected. + --- @@ -111,11 +116,14 @@ point = collapse(pf, np.mean) # (N, 1) fra # WRONG: list-in-cell / object dtype (the measured non-scaler) — raises PredictionFrame(y_pred=np.array(list_of_lists, dtype=object), index=idx) -# WRONG: mutating the value buffer in place. Immutable *by convention* — NOT -# write-protected (the buffer stays writeable for zero-copy), so this does NOT raise; -# it silently corrupts every frame sharing the buffer (e.g. via with_metadata). -# Build a new frame instead. (ADR-025 / register C-63; the index IS write-protected.) -pf.values[:] = 0 # unsupported: silent shared-buffer corruption, no error +# WRONG: mutating the value buffer in place. Since 2.0.0 this RAISES — the buffer is +# write-protected (ADR-028 / register C-66). Until then it silently corrupted every +# frame sharing the buffer, e.g. via with_metadata. Build a new frame instead. +pf.values[:] = 0 # ValueError: assignment destination is read-only + +# WRONG: passing something that is not an index. Raises since 2.0.0; before that it +# was accepted silently, because only `index.n_rows` was ever read (ADR-028). +PredictionFrame(y_pred=arr, index=some_other_frame) # TypeError ``` --- @@ -131,7 +139,10 @@ pf.values[:] = 0 # unsupported: silent shared-buffer corruption, no err C-80). `with_metadata` allocates no second `values` buffer — that one *is* measured (`tests/test_properties.py::test_with_metadata_shares_the_values_buffer`, C-07). - **Red:** object-dtype / wrong-dtype / NaN-identifier construction raises; - no-pandas import-enforcement. + a non-`SpatioTemporalIndex` `index` raises, including a frame and a bare object with + only `n_rows` — `tests/test_construction_red.py`; `reindex`/`reindex_fill` raise on a + non-index argument — `tests/test_select.py`; the value buffer is read-only while the + caller's array is not — `tests/test_properties.py`; no-pandas import-enforcement. --- diff --git a/docs/CICs/SpatioTemporalIndex.md b/docs/CICs/SpatioTemporalIndex.md index bd2a827..524b59b 100644 --- a/docs/CICs/SpatioTemporalIndex.md +++ b/docs/CICs/SpatioTemporalIndex.md @@ -89,7 +89,14 @@ Assumptions that do not hold **must raise** at construction (ADR-009), never fal `SpatialLevel`. - `cross_level_align` raises if called without an injected mapping — it must **never** silently fetch or assume one (the defining boundary of ADR-014). -- Nothing fails silently. +- Raises `TypeError` when a same-level binary op is handed something that is not a + `SpatioTemporalIndex` — `searchsorted`/`reindex`, `is_superset_of` and `intersect` all + route through one guard in `_require_same_level` (since 2.0.0, ADR-028). +- Nothing fails silently. **That claim was false until 2026-08-18**: the same-level ops + read `other._level` unchecked, so a non-index argument raised + `AttributeError: '...' object has no attribute '_level'` — a private attribute of a + class the caller never named, which ADR-008 does not count as failing loud. It is true + again now, and `tests/test_select.py` pins it rather than leaving it asserted. --- diff --git a/docs/CICs/Summarize.md b/docs/CICs/Summarize.md index 2b694a6..c2d69d0 100644 --- a/docs/CICs/Summarize.md +++ b/docs/CICs/Summarize.md @@ -219,6 +219,19 @@ re-derive (ADR-017). - A reducer producing the wrong shape fails loud via the rebuilt frame's validation. - `aggregate_distributions` without a mapping is an error (mirrors `cross_level_align`). +- `map_estimate` raises `ValueError` on non-finite draws (`NaN`/`±inf`) as of 2.0.0 — the + same guard `exceedance` and `expected_shortfall` have carried since v1.5.0/v1.6.0. + Before that it overflowed the integer bin index and crashed with a bare `IndexError` + naming neither the cause nor the caller (register C-57, shipped as a rider on ADR-028). +- **`assert_summarizer_contract` asserts its frame is structurally sound before asserting + anything about the summarizers.** Every estimator here reads `frame.values` and + `frame.n_rows` and none reads `frame.index`, so until 2.0.0 the checker returned cleanly + for a frame whose index was not an index at all — a published checker issuing a **false + pass**, which `docs/CICs/Conformance.md` names as the failure mode this family must never + have. Construction now rejects such a frame, so the assertion is unreachable by ordinary + means; it is kept because consumers run this suite against their own frame factories + (ADR-016) and `with_metadata` already builds frames through `__new__` rather than + `__init__` (ADR-028, falsify audit 2026-08-18). - **Silent caveat, not a loud failure (register C-32):** `map_estimate`'s lowest-index tie-break is statistically biased toward zero on right-skewed, zero-inflated, low-sample posteriors. It does not raise — it returns a defensible-but-biased mode; consumers must diff --git a/docs/CICs/TargetFrame.md b/docs/CICs/TargetFrame.md index 75cba29..fafff03 100644 --- a/docs/CICs/TargetFrame.md +++ b/docs/CICs/TargetFrame.md @@ -35,8 +35,9 @@ boundary array-native, replacing the pandas actuals the eval adapter takes today trailing sample axis is explicit with `S == 1` (ADR-012); no object dtype; identifiers integer, length-`N`, complete. - Immutable with the same copy-vs-view semantics as the other frames (register C-07): - the **index is enforced** read-only; the **value buffer is immutable by convention** - (writeable for zero-copy — in-place `.values` mutation is unsupported; ADR-025 / C-63). + the **index is enforced** read-only, and **since 2.0.0 so is the value buffer** — a + read-only *view*, preserving zero-copy and `mmap` while leaving the caller's array + writeable (ADR-028 / C-66; convention-only until then, ADR-025 / C-63). - Carries a typed `metadata` header (ADR-013) and the same row/metadata surface as the sibling frames: `with_metadata`, `select(positions | mask)`, `reindex(other)` (raises unless this index is a superset of `other`), and the dense-grid companion @@ -101,9 +102,12 @@ TargetFrame(y_true=actuals_1d, index=idx) # raises # WRONG: treating it as a sampled frame and asking for many quantiles -# WRONG: mutating the value buffer in place — immutable *by convention*, not -# write-protected, so it does NOT raise; build a new frame (ADR-025 / register C-63). -tf.values[:] = 0 # unsupported: silent shared-buffer corruption, no error +# WRONG: mutating the value buffer in place — write-protected since 2.0.0, so this +# now raises rather than silently corrupting buffer-sharing frames (ADR-028 / C-66). +tf.values[:] = 0 # ValueError: assignment destination is read-only + +# WRONG: passing something that is not an index — raises since 2.0.0 (ADR-028) +TargetFrame(y_true=actuals, index=some_other_frame) # TypeError ``` --- @@ -117,8 +121,10 @@ tf.values[:] = 0 # unsupported: silent shared-buffer corruption, no err - **Beige:** serves through the same protocol surface as `PredictionFrame` (`is_sample == False`) — `tests/test_falsification_twin_parity.py`; copy-vs-view — `tests/test_properties.py`. -- **Red:** `(N,)` or `(N, S>1)` input raises — `tests/test_construction_red.py`; - no-pandas import-enforcement — `tests/test_import_enforcement.py`. +- **Red:** `(N,)` or `(N, S>1)` input raises, and a non-`SpatioTemporalIndex` `index` + raises — `tests/test_construction_red.py`; the value buffer is read-only — + `tests/test_properties.py`; no-pandas import-enforcement — + `tests/test_import_enforcement.py`. (This section named no pinning test file until 2026-08-18, where the other CICs name one to five — register C-80.) diff --git a/pyproject.toml b/pyproject.toml index fc25ac5..44718c7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "views-frames" -version = "1.11.0" +version = "2.0.0" description = "The VIEWS platform data-contract layer: immutable array+identifier frames (numpy only, root of the dependency DAG)." authors = [ { name = "Simon Polichinel von der Maase", email = "simmaa@prio.org" }, diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index a2c3e60..fc1c586 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -5,9 +5,9 @@ | Project | views-frames | | Owner | VIEWS platform maintainers | | Last Updated | 2026-08-18 | -| Total Concerns | 89 | -| Open Concerns | 11 | -| Resolved Concerns | 78 | +| Total Concerns | 92 | +| Open Concerns | 10 | +| Resolved Concerns | 82 | | Disagreements | 12 | --- @@ -96,7 +96,7 @@ None of these is a defect to fix now. Every one requires someone to work around |-------|-------| | ID | C-13 | | Tier | 3 (recalibrated from 2 on 2026-07-31, `review-rr` strategic) | -| Status | **awaiting** — the next MAJOR bump (this entry *is* its pre-tag checklist) | +| Status | **awaiting** — the next MAJOR bump **after 2.0.0** (this entry *is* its pre-tag checklist; it was executed once, on 2026-08-18 — see the execution record below) | | Source | expert-review (2026-06-20) | | Trigger | **When a MAJOR bump is opened — before tagging:** confirm every consumer repo has an adoption issue filed and a pinned floor per GOVERNANCE §coordinated-bump, and pair the bump with the **freeze-cluster rider list** (C-66's `setflags` enforce + its red test, C-57's `np.isfinite` guard on `map_estimate`, C-43's shared-binning extraction) — a MAJOR that fans out without carrying its riders spends the coordination budget for nothing. | | Location | `README.md` §12 (~12 register items, 3+ repos); `GOVERNANCE.md` (coordinated-bump process) | @@ -104,7 +104,29 @@ None of these is a defect to fix now. Every one requires someone to work around The leaf's breadth is both its value and an inherent concentration risk (critique_01 §3.7): it is structurally the single point every consumer pins. **Mitigation shipped** — a minimal, stable, **frozen v1.0.0** (ADR-018) gives consumers a contract that will not churn, and ADR-016 / GOVERNANCE name the owner and the coordinated MAJOR-bump process (C-05, C-10 resolved). **Residual is accepted and monitored:** the fan-out cost of any future MAJOR is irreducible; the control is the GOVERNANCE process, watched as consumers adopt. -**Tier 3, recalibrated 2026-07-31.** Originally Tier 2, but Tier 2 means *structural fragility that will cause failures under realistic change scenarios* — this entry describes an irreducible **cost** incurred under a process that already exists and has never failed, not a fragility. It is an **accepted standing condition** kept in Open solely so the coordinated-bump discipline stays visible, and its trigger now does real work: it is the pre-tag checklist for the one event that makes concentration bite. See also D-06. +**Tier 3, recalibrated 2026-07-31.** Originally Tier 2, but Tier 2 means *structural fragility that will cause failures under realistic change scenarios* — this entry describes an irreducible **cost** incurred under a process that already exists and has never failed, not a fragility. It is an **accepted standing condition** kept in Open solely so the coordinated-bump +discipline stays visible, and its trigger now does real work: it is the pre-tag checklist for +the one event that makes concentration bite. See also D-06. + +**Execution record — 2.0.0, 2026-08-18 (ADR-028).** The first time this checklist ran. What +it produced: + +- **Riders carried:** C-66 (value-buffer write-protection, though as a read-only view rather + than the recorded one-liner — see that entry) and C-57 (`map_estimate`'s non-finite guard). +- **Rider declined, in writing:** C-43. The reason is recorded in that entry and its + precondition rewritten, because "#89 or a MAJOR" silently became false the moment 2.0.0 + tagged. +- **Fan-out measured, not assumed:** three consumers are lockfile-pinned and constrained + `<2` — views-faoapi (`uv.lock`), views-postprocessing (`poetry.lock`), views-crafdapi + (`uv.lock`). All three reach 2.0.0 only by changing the constraint and re-locking. +- **`CONFORMANCE_FLOOR` moved `1.0.0` → `2.0.0`** — the first move since the freeze, per + GOVERNANCE's MAJOR process step 3. +- **Adoption issues:** the maintainer's action; this repo cannot open issues in sibling + repos. The checklist's "confirm every consumer repo has an adoption issue filed" is what + gates tagging, not merging. + +The checklist earned its keep: it is what caused C-43 to be declined explicitly rather than +forgotten, and what turned "the consumers will pick it up" into three named lockfiles. --- @@ -152,12 +174,32 @@ Note the estimator is **already semi-parametric**: the `zero_mass_threshold` rul |-------|-------| | ID | C-43 | | Tier | 4 | -| Status | **awaiting** — #89 or a MAJOR; `point.py` is frozen and ulp-sensitive (C-24) | +| Status | **awaiting** — #89. (Was "#89 or a MAJOR" until 2026-08-18: the 2.0.0 MAJOR came and **declined this rider on purpose** — see below — so a MAJOR alone no longer discharges it.) | | Source | tech-debt-cleanup (2026-06-24) | | Trigger | When `map_estimate` is unfrozen or reworked (#89), or the bimodality binning needs to change — at that point extract a shared row-blocked binning helper. It is **not** safely de-dupable now: `point._batched_map` is frozen (ADR-018) and its bin edges are ~1-ulp-sensitive across numpy versions (the C-24 portability saga), so touching it risks a behaviour change to `map_estimate`. | | Location | `src/views_frames_summarize/bimodality.py` (`_coarse_counts`); `src/views_frames_summarize/point.py` (`_batched_map`). | -Both functions implement per-row histogram binning over a row-block. `_coarse_counts` (v1.1.0) is a deliberately simplified clipped-linear bucket for a heuristic flag; `_batched_map` (frozen v1.0.0) reproduces `numpy.histogram`'s edge-exact path bit-for-bit for the MAP. The two are **independently correct and tested** — the "debt" is the maintenance cost of two binning implementations to keep mentally aligned. **Tier 4** — no correctness or reliability impact; bounded because `point.py` is frozen and won't drift. Intentionally **not** unified now (extracting a shared helper would touch frozen, C-24-ulp-sensitive code — a stability risk the tech-debt protocol says to defer). See C-24 (resolved — the binning portability constraint), ADR-018 (the freeze that blocks the fix), #89. +Both functions implement per-row histogram binning over a row-block. `_coarse_counts` (v1.1.0) is a deliberately simplified clipped-linear bucket for a heuristic flag; `_batched_map` (frozen v1.0.0) reproduces `numpy.histogram`'s edge-exact path bit-for-bit for the MAP. The two are **independently correct and tested** — the "debt" is the maintenance cost of two binning implementations to keep mentally aligned. **Tier 4** — no correctness or reliability impact; bounded because `point.py` is frozen and won't drift. Intentionally **not** unified now (extracting a shared helper would touch frozen, +C-24-ulp-sensitive code — a stability risk the tech-debt protocol says to defer). See C-24 +(resolved — the binning portability constraint), ADR-018 (the freeze that blocks the fix), #89. + +**Declined as a rider on the 2.0.0 MAJOR (2026-08-18, ADR-028).** C-13's pre-tag checklist +names this entry on the freeze-cluster rider list, so passing it over silently would be the +"claimed enforcement that does not exist" failure this register has recorded three times. +The reason it was declined: + +The two functions are not two implementations of one concept. `_coarse_counts` is a clipped +linear bucket, deliberately approximate, sized for a heuristic flag. `_batched_map` +reproduces `numpy.histogram`'s edge-exact path bit-for-bit and is ~1-ulp sensitive across +numpy versions. Extracting a shared helper forces one onto the other's path, which changes +the output of either `bimodality` or the frozen `map_estimate` — a behaviour change bought +for a Tier-4 entry this register itself describes as having no correctness or reliability +impact. WET-before-DRY asks whether the shared concept is real; here it is superficial. + +The MAJOR that this entry was half-waiting for has now been spent, and spending it again is +not cheap, so the precondition is #89 alone: when `map_estimate` is reworked there, the +edge-exact constraint is on the table anyway and the extraction becomes free rather than +risky. --- @@ -177,22 +219,6 @@ Under Option B (the ratified boundary; C-01), `MetricFrame` lives in `views-eval --- -### C-57: `map_estimate` raises an obscure `IndexError` (not a clean error) on ±inf draws - -| Field | Value | -|-------|-------| -| ID | C-57 | -| Tier | 3 | -| Status | **awaiting** — a cross-estimator non-finite hardening pass (additive MINOR) | -| Source | falsify audit (2026-06-25, P5b — discovered while widening the exceedance/ES guards) | -| Trigger | When a consumer feeds a frame containing an `inf` draw (a valid float32 the leaf does **not** ban — e.g. an upstream model bug) to the frozen `map_estimate`: the histogram span is `inf`, the bin index divides to `nan`, and the `astype(intp)` cast overflows to the int-min sentinel, so `np.take_along_axis` raises `IndexError: index -9223372036854775808 is out of bounds` instead of a clean `ValueError` or a finite result. | -| Location | `src/views_frames_summarize/point.py::_batched_map` (the `astype(np.intp)` cast feeding `np.take_along_axis`) (`_batched_map`). | -| Cross-refs | ADR-018 (frozen v1 surface — behavior is locked), C-50/C-56 (the new estimators now fail loud cleanly on non-finite via `np.isfinite`; `map_estimate` is the frozen sibling that does **not**), ADR-008 (fail-loud posture). | - -The frozen surface is **inconsistent** on non-finite draws: `collapse(np.mean)` propagates `inf` (visible), the new `exceedance`/`expected_shortfall` now **fail loud** on it (C-50/C-56), but `map_estimate` **crashes with an obscure `IndexError`** rather than a clean, actionable error. This is **not** silent corruption (it is loud, and `inf` draws are out-of-contract upstream bugs), so it is **not** a publish blocker for v1.6.0 — and `map_estimate`'s behavior is **locked by the ADR-018 freeze**, so it cannot change without an additive hardening pass. **Tier 3** — ungraceful failure on a leaf-permitted input; a future cross-estimator non-finite hardening (a reserved additive MINOR) should give `map_estimate` the same clean `np.isfinite` guard. **Open** — watch-item, no fix shipped in v1.6.0. - ---- - ### C-62: `reconcile_proportional` is an information-losing per-draw approximation (no joint-calibration guarantee) | Field | Value | @@ -209,22 +235,6 @@ The frozen surface is **inconsistent** on non-finite draws: `collapse(np.mean)` --- -### C-66: value-buffer write-protection is deferred to the next MAJOR (the C-63 enforce-rider) - -| Field | Value | -|-------|-------| -| ID | C-66 | -| Tier | 3 | -| Status | **awaiting** — the next MAJOR (the one-line enforce + red test are pre-written) | -| Source | review-diff + register-risk (2026-06-28, epic #179 / S2) — the residual of the C-63 resolution-by-decision (ADR-025). | -| Trigger | When a MAJOR bump is opened for **any** reason — add `self._values.setflags(write=False)` after the `self._values = ...` assignment in the three frame constructors (`prediction_frame.py::PredictionFrame.__init__`, `target_frame.py::TargetFrame.__init__`, `feature_frame.py::FeatureFrame.__init__`) **and** a red test (`frame.values.flags.writeable is False`; mirror `tests/test_properties.py::test_with_metadata_shares_the_values_buffer`), riding that MAJOR for free. **Or** sooner, if a consumer is found applying an in-place `.values` mutation (`frame.values[mask] = 0`, `*=`, a clamp) on a `with_metadata`/`select` buffer-sharing frame — promote/expedite the enforce then. | -| Location | the `self._values = values` assignment in each of `src/views_frames/prediction_frame.py::PredictionFrame.__init__`, `target_frame.py::TargetFrame.__init__` and `feature_frame.py::FeatureFrame.__init__` (bare assignment, no `setflags`); `src/views_frames/_validation.py::coerce_values` (`coerce_values` returns float32 without copy); contrast `src/views_frames/index.py::SpatioTemporalIndex.__init__` (the two `setflags(write=False)` calls) (the index **is** write-protected). Decision in `docs/ADRs/025_value_buffer_immutability_by_convention.md`. | -| Cross-refs | **C-63** (RESOLVED by contract correction — this entry tracks the *deferred enforce* it left open), **ADR-025** (the decision + the exact one-line-per-constructor change), ADR-018 (`values` is frozen-surface, so the enforce is a MAJOR), GOVERNANCE.md (SemVer: "tightening an invariant" = MAJOR), C-07 (the zero-copy reason the buffer is left writeable). | - -C-63 was resolved by **correcting the contract** (ADR-025): the value buffer is documented as immutable *by convention* and the docs no longer claim an unenforced guarantee. But the **code** is unchanged — `frame.values.flags.writeable` is still `True`, and `with_metadata` shares the buffer — so the underlying mechanism (an in-place `.values` mutation **silently corrupts every frame sharing the buffer**, the Tier-2 basis of C-63) is **mitigated, not removed**. The mitigation is documentation (three frame CICs §9 + README design principle 3 say it is unsupported) + the empirical fact that **nothing in `src/` or `tests/` mutates `.values`**. The actual write-protection (`setflags(write=False)`) is deliberately deferred because, on the frozen-surface `values`, it is a **MAJOR** (GOVERNANCE/ADR-018) and does not justify a standalone cross-repo coordinated bump. **Tier 3** — this entry tracks the *accepted deferral* of a documented-and-unexercised exposure (the acute silent-corruption path requires a consumer to ignore the published contract); it is a governance/safety-tracking item, not a current defect, and exists so the deferred enforce stays visible in the **Open** section rather than buried in a resolved entry. **Open** — until the enforce rides the next MAJOR. - ---- - ### C-71: dense-grid fill allocates the full dense buffer — grid-scale memory footgun | Field | Value | @@ -257,6 +267,38 @@ The fill primitive makes densification a one-liner, which is the point (#203, fa --- +### C-95: `SpatioTemporalIndex` write-protects the caller's identifier arrays, and its comment says otherwise + +| Field | Value | +|-------|-------| +| ID | C-95 | +| Tier | 4 | +| Status | **awaiting** — the next MAJOR after 2.0.0 (changing it is a behaviour change on frozen surface; the comment half is already corrected) | +| Source | falsify audit follow-up (2026-08-18) — found while shipping C-66, which had to solve the same problem for the frames | +| Trigger | When a consumer reports that constructing a `SpatioTemporalIndex` made their own `time`/`unit` array read-only — or, sooner, when the next MAJOR is opened for another reason, at which point aligning the index with the frames is a two-line rider. | +| Location | `src/views_frames/index.py::SpatioTemporalIndex.__init__` (the two `setflags(write=False)` calls after `np.ascontiguousarray`). | +| Cross-refs | C-66 (the frames' version of the same problem, solved with a read-only view), C-07 (the zero-copy reason the input is not copied), ADR-025, ADR-028. | + +`np.ascontiguousarray` returns the **same object** when its input is already contiguous, so +`self._time.setflags(write=False)` also write-protects the caller's array. Constructing an +index silently makes the arrays you passed in read-only, for the rest of their life. + +This is exactly the hazard C-66 had to avoid for the frames, where it matters more (a value +buffer is larger and likelier to be reused across batches). The frames now take a read-only +*view*; the index still does not, so the two now differ. + +**Tier 4, and deliberately left alone.** No consumer has reported it, identifier arrays are +small and rarely reused, and the behaviour has been shipped since v0.1.0 — so changing it is +a behaviour change on frozen surface, i.e. another MAJOR, for a hole nothing is known to hit. +That is ADR-025's reasoning applied consistently rather than an exception made for +convenience. + +**The half that *was* fixed in 2.0.0 is the comment.** It read "store as read-only views", +which is not what `ascontiguousarray` returns for contiguous input — a false statement in +code, costing nothing to correct. It now says what the code does and points here. + +--- + ## Disagreements ### D-01: `SpatioTemporalIndex` domain-purity fork (where does cross-level alignment live?) @@ -844,6 +886,82 @@ The beige-marker item is the one **not** done: applying 🟨 across the suite me --- +### C-93: a frame's `index` was never type-checked, and the published checker certified the result — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-93 | +| Tier | 2 | +| Resolved | 2026-08-18 | +| Resolution | Fixed in 2.0.0 (ADR-028): the three constructors raise `TypeError` on a non-`SpatioTemporalIndex` `index`, and `assert_summarizer_contract` asserts its frame's index is real. | +| Source | falsify audit (2026-08-18) — an unpredicted finding, surfaced while correcting my own caller error in a probe | +| Cross-refs | ADR-028 (the MAJOR), ADR-008 (validation in `__init__`), ADR-016 (why a false pass is a cross-repo defect), ADR-011 (why the guard is written three times), C-66 (the rider it shipped beside), C-94. | + +Construction validated `y_pred` four times — coerced it, checked dtype, ndim, row count — and +never checked `index` at all. It read exactly one attribute off it, `n_rows`, and a frame has +one. So `PredictionFrame(values, another_frame)` constructed silently, as did any object +exposing `n_rows`: + +``` +PredictionFrame(v, a_frame) -> constructed, no error +FeatureFrame(v, a_frame) -> constructed, no error +TargetFrame(v, a_frame) -> constructed, no error +PredictionFrame(v, obj_with_only_n_rows) -> constructed, no error +``` + +**Tier 2 because of the second half.** Every summarizer reads `.values` and `.n_rows` and +none reads `.index`, so the malformed frame produced *numerically correct* answers from +`collapse`, `map_estimate` and `hdi` — and `assert_summarizer_contract` **returned None for +it**. That checker is published under ADR-016 and consumers run it in their own CI, and +`docs/CICs/Conformance.md` names "a checker that cannot detect a violation" as the failure +mode this module must never have. A false pass in a contract checker is worse than no +checker, because it is evidence a consumer is entitled to rely on. + +Not Tier 1: nothing is silently *corrupted*. The frame fails as soon as anything touches the +index — it fails late and far from the cause, not never. + +The guard is written three times rather than extracted. `_validation.py` is imported **by** +`index.py`, so it cannot import `SpatioTemporalIndex` back without a cycle that +`tests/test_import_enforcement.py` and the import-linter contracts exist to prevent; and +ADR-011 Option C already accepts WET across the three siblings. A `Protocol` check was +rejected on the facts: `SpatioTemporalIndexed` describes a *frame*, `SpatioTemporalIndex` +does not even satisfy it (no `.index`), and `runtime_checkable` checks member presence only +— the exact weak duck-type that caused this. + +The checker keeps its own assertion despite construction making it unreachable by ordinary +means, because `with_metadata` already builds frames through `__new__`, and consumers run +the suite against their own factories. + +--- + +### C-94: same-level alignment leaked a private attribute instead of failing loud — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-94 | +| Tier | 3 | +| Resolved | 2026-08-18 | +| Resolution | Fixed in 2.0.0 (ADR-028): one `TypeError` guard at the top of `SpatioTemporalIndex._require_same_level`, which every same-level binary op routes through. | +| Source | falsify audit (2026-08-18) | +| Cross-refs | ADR-008 (fail loud, `ValueError`/`TypeError`), ADR-028, C-93 (the same audit; the same missing type check one layer down). | + +`_require_same_level` read `other._level` with no type check, so `reindex`, `reindex_fill`, +`is_superset_of` and `intersect` — and the frame-level `reindex`/`reindex_fill` that delegate +to them — raised `AttributeError: 'PredictionFrame' object has no attribute '_level'`. + +That names a **private attribute of a class the caller never mentioned**. ADR-008 requires +`ValueError`/`TypeError` at every validation guard, and `docs/CICs/SpatioTemporalIndex.md` §6 +claimed "Nothing fails silently" — true of the *values*, false here in the sense that +mattered: the error told the caller nothing about what they did wrong. + +**Tier 3, not 2** — it always failed, and immediately. Only the diagnostic was bad. + +One guard fixes the family because all four ops already funnel through this one private +helper. That was worth checking rather than assuming: the alternative reading was four +guards in four methods, which would have been four places to drift. + +--- + ### C-92: eleven CI checks ran and none was required — RESOLVED | Field | Value | @@ -893,6 +1011,73 @@ applying are different claims, and only the second one is the guarantee. --- +### C-66: value-buffer write-protection is deferred to the next MAJOR — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-66 | +| Tier | 3 | +| Resolved | 2026-08-18 | +| Resolution | Shipped in 2.0.0 (ADR-028), riding the MAJOR exactly as this entry planned — but as a read-only **view**, not the `setflags` one-liner ADR-025 recorded. | +| Source | review-diff + register-risk (2026-06-28, epic #179 / S2) — the residual of the C-63 resolution-by-decision (ADR-025). | +| Cross-refs | **C-63** (RESOLVED by contract correction — this entry tracks the *deferred enforce* it left open), **ADR-025** (the decision + the exact one-line-per-constructor change), ADR-018 (`values` is frozen-surface, so the enforce is a MAJOR), GOVERNANCE.md (SemVer: "tightening an invariant" = MAJOR), C-07 (the zero-copy reason the buffer is left writeable). | + +The 2.0.0 MAJOR happened for another reason (the index type guard, ADR-028), so this rode +it for free — which is precisely the outcome this entry was written to wait for. + +**It did not ship as written, and the difference matters.** ADR-025 and this entry both +record the fix as `self._values.setflags(write=False)` after the assignment. That would have +been a defect. `coerce_values` returns the caller's *own* array when it is already `float32` +— the C-07 zero-copy guarantee — so the one-liner silently makes the **caller's** array +read-only as well. Measured rather than reasoned about: + +``` +caller = np.zeros((2,3), np.float32) +arr = np.asanyarray(caller) # what coerce_values returns +arr.setflags(write=False) +caller.flags.writeable -> False # action at a distance, outside this contract +``` + +The frames take `values.view()` and lock the view instead. Verified: the frame's buffer is +read-only, the caller's array stays writeable, `np.shares_memory` is still True, and a +`np.memmap` keeps its subclass and its zero-copy through `.view()`. + +Pinned by three tests in `tests/test_properties.py` — the buffer is read-only for all three +frames, the caller's array is not, and the buffer is still shared. The middle one is the one +that would have failed under the one-liner. + +Left standing on purpose: `SpatioTemporalIndex` still write-protects its identifier arrays +in place and so *does* reach into the caller's arrays. Long-standing behaviour, out of +2.0.0's scope, now recorded as **C-95**. + +--- + +### C-57: `map_estimate` raises an obscure `IndexError` on ±inf draws — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-57 | +| Tier | 3 | +| Resolved | 2026-08-18 | +| Resolution | Shipped in 2.0.0 as a rider on ADR-028: `map_estimate` now raises `ValueError` on non-finite draws, the same guard `exceedance` and `expected_shortfall` carry. | +| Source | falsify audit (2026-06-25, P5b — discovered while widening the exceedance/ES guards) | +| Cross-refs | ADR-018 (frozen v1 surface — behavior is locked), C-50/C-56 (the new estimators now fail loud cleanly on non-finite via `np.isfinite`; `map_estimate` is the frozen sibling that does **not**), ADR-008 (fail-loud posture). | + +`map_estimate` was the last estimator without the `np.isfinite` guard, because `point.py` +is frozen surface (ADR-018) and this entry was therefore waiting on a release that touched +it. Named on C-13's freeze-cluster rider list; 2.0.0 carried it. + +Nothing that succeeded before now fails: an `inf` draw already crashed, via a `nan` bin +index whose `astype(intp)` cast overflowed to the int-min sentinel and produced +`IndexError: index -9223372036854775808 is out of bounds`. That named neither the cause nor +the caller. It now raises `ValueError` naming both. + +This is why C-57 was classifiable as additive while C-66 was not — the test this register +applies is *"does any currently-succeeding call start raising?"*, and here the answer was +always no. Pinned in `tests/test_summarize_estimators.py` for `inf`, `-inf` and `NaN`. + +--- + ### C-74: the CI gate was a strict subset of the local gate — `validate_docs.sh` and `ruff format` ran only by habit — RESOLVED | Field | Value | diff --git a/src/views_frames/conformance/__init__.py b/src/views_frames/conformance/__init__.py index 8a41282..a8264b8 100644 --- a/src/views_frames/conformance/__init__.py +++ b/src/views_frames/conformance/__init__.py @@ -19,7 +19,8 @@ assert_frame_envelope(my_metric_frame()) The floor is governed in ``GOVERNANCE.md``; ``CONFORMANCE_FLOOR`` records the version -this suite belongs to. +this suite belongs to. It moved to ``2.0.0`` with the 2.0.0 MAJOR (ADR-028) — the first +move since the v1.0.0 freeze. """ from __future__ import annotations @@ -29,7 +30,11 @@ import numpy as np -CONFORMANCE_FLOOR = "1.0.0" +# Bumped to 2.0.0 on 2026-08-18 — the FIRST move since the freeze. `assert_summarizer_ +# contract` now rejects a frame whose index is not an index, which narrows what the +# published suite accepts, and GOVERNANCE bumps the floor on any breaking change to any +# published entry point. A consumer reading this learns which contract its CI asserts. +CONFORMANCE_FLOOR = "2.0.0" __all__ = [ "CONFORMANCE_FLOOR", diff --git a/src/views_frames/feature_frame.py b/src/views_frames/feature_frame.py index d23a531..1085d2a 100644 --- a/src/views_frames/feature_frame.py +++ b/src/views_frames/feature_frame.py @@ -41,6 +41,10 @@ def __init__( f"trailing sample axis (ADR-012), got ndim={values.ndim}. " "Use FeatureFrame.from_2d to build a frame from a 2-D (N, F) array." ) + if not isinstance(index, SpatioTemporalIndex): + raise TypeError( + f"index must be a SpatioTemporalIndex, got {type(index).__name__}" + ) if values.shape[0] != index.n_rows: raise ValueError( f"y_features has {values.shape[0]} rows but index has {index.n_rows}" @@ -50,7 +54,13 @@ def __init__( f"feature_names length ({len(feature_names)}) must match the " f"feature axis ({values.shape[1]})" ) - self._values = values + # A read-only *view*, not `values.setflags(write=False)` (register C-66). + # `coerce_values` returns the caller's own array when it is already float32 + # (the C-07 zero-copy guarantee), so locking it in place would silently make + # the CALLER's array read-only — action at a distance well outside this + # contract. The view shares the buffer, so zero-copy and mmap are preserved. + self._values: NDArray[np.float32] = values.view() + self._values.setflags(write=False) self._index = index self._feature_names = list(feature_names) self._metadata = metadata if metadata is not None else FrameMetadata() diff --git a/src/views_frames/index.py b/src/views_frames/index.py index 57c534f..7689953 100644 --- a/src/views_frames/index.py +++ b/src/views_frames/index.py @@ -47,7 +47,11 @@ def __init__( is_array = isinstance(time, np.ndarray) and time.ndim >= 1 n = int(time.shape[0]) if is_array else -1 validate_identifiers({"time": time, "unit": unit}, n_rows=n) - # store as read-only views so the value object cannot be mutated in place + # Write-protect the identifier arrays so the value object cannot be mutated + # in place. Note `ascontiguousarray` returns the *same* array when the input + # is already contiguous, so this also write-protects the caller's array. That + # is long-standing index behaviour, deliberately left alone here; the frames + # take a read-only view instead, for the reason given in their constructors. self._time = np.ascontiguousarray(time) self._unit = np.ascontiguousarray(unit) self._time.setflags(write=False) @@ -173,6 +177,16 @@ def _row_view(keys: NDArray[np.int64]) -> NDArray[np.void]: ) def _require_same_level(self, other: SpatioTemporalIndex) -> None: + # Type first. Every same-level binary op (`searchsorted`/`reindex`, + # `is_superset_of`, `intersect`) routes through here, so this one guard is + # what stops a non-index argument reaching `other._level` and leaking + # `AttributeError: '...' object has no attribute '_level'` — a private + # attribute of a class the caller never named. ADR-008 requires a + # ValueError/TypeError at every validation guard (falsify audit 2026-08-18). + if not isinstance(other, SpatioTemporalIndex): + raise TypeError( + f"expected a SpatioTemporalIndex, got {type(other).__name__}" + ) if self._level != other._level: raise ValueError( "same-level operation requires equal SpatialLevel; " diff --git a/src/views_frames/prediction_frame.py b/src/views_frames/prediction_frame.py index c842307..b0778e4 100644 --- a/src/views_frames/prediction_frame.py +++ b/src/views_frames/prediction_frame.py @@ -37,11 +37,21 @@ def __init__( raise ValueError( f"PredictionFrame y_pred must be 2D (N, S), got ndim={values.ndim}" ) + if not isinstance(index, SpatioTemporalIndex): + raise TypeError( + f"index must be a SpatioTemporalIndex, got {type(index).__name__}" + ) if values.shape[0] != index.n_rows: raise ValueError( f"y_pred has {values.shape[0]} rows but index has {index.n_rows}" ) - self._values = values + # A read-only *view*, not `values.setflags(write=False)` (register C-66). + # `coerce_values` returns the caller's own array when it is already float32 + # (the C-07 zero-copy guarantee), so locking it in place would silently make + # the CALLER's array read-only — action at a distance well outside this + # contract. The view shares the buffer, so zero-copy and mmap are preserved. + self._values: NDArray[np.float32] = values.view() + self._values.setflags(write=False) self._index = index self._metadata = metadata if metadata is not None else FrameMetadata() diff --git a/src/views_frames/target_frame.py b/src/views_frames/target_frame.py index 2a83e25..2146ecf 100644 --- a/src/views_frames/target_frame.py +++ b/src/views_frames/target_frame.py @@ -36,11 +36,21 @@ def __init__( "TargetFrame y_true must have shape (N, 1) with an explicit " f"trailing axis (ADR-012), got shape {values.shape}" ) + if not isinstance(index, SpatioTemporalIndex): + raise TypeError( + f"index must be a SpatioTemporalIndex, got {type(index).__name__}" + ) if values.shape[0] != index.n_rows: raise ValueError( f"y_true has {values.shape[0]} rows but index has {index.n_rows}" ) - self._values = values + # A read-only *view*, not `values.setflags(write=False)` (register C-66). + # `coerce_values` returns the caller's own array when it is already float32 + # (the C-07 zero-copy guarantee), so locking it in place would silently make + # the CALLER's array read-only — action at a distance well outside this + # contract. The view shares the buffer, so zero-copy and mmap are preserved. + self._values: NDArray[np.float32] = values.view() + self._values.setflags(write=False) self._index = index self._metadata = metadata if metadata is not None else FrameMetadata() diff --git a/src/views_frames_summarize/conformance.py b/src/views_frames_summarize/conformance.py index b7cafb6..336618c 100644 --- a/src/views_frames_summarize/conformance.py +++ b/src/views_frames_summarize/conformance.py @@ -9,6 +9,7 @@ import numpy as np +from views_frames import SpatioTemporalIndex from views_frames_summarize import config from views_frames_summarize._common import AnyFrame from views_frames_summarize.bimodality import bimodality @@ -44,6 +45,23 @@ def assert_summarizer_contract(frame: AnyFrame) -> None: AssertionError: a summarizer violates its output contract. """ _require_assertions() + + # The frame must be structurally sound before any law about it means anything. + # Every summarizer below reads `frame.values` and `frame.n_rows` and none reads + # `frame.index`, so before 2.0.0 this suite returned cleanly for a frame whose + # index was not an index at all — a published checker issuing a false pass, which + # `docs/CICs/Conformance.md` names as the failure mode this module must never + # have (falsify audit 2026-08-18). + # + # Construction now rejects such a frame, so this is unreachable through the + # ordinary constructors. It is kept because consumers run this suite against + # *their own* frame factories under ADR-016, and `with_metadata` already builds + # frames through `Frame.__new__` rather than `__init__` — so a bypass path exists + # today and more may be added. + assert isinstance(frame.index, SpatioTemporalIndex), ( + "frame.index must be a SpatioTemporalIndex, got " + f"{type(frame.index).__name__} — the frame misreports itself" + ) n = frame.n_rows point = collapse(frame, np.mean) diff --git a/src/views_frames_summarize/point.py b/src/views_frames_summarize/point.py index a9d2fcf..2712010 100644 --- a/src/views_frames_summarize/point.py +++ b/src/views_frames_summarize/point.py @@ -44,6 +44,14 @@ def map_estimate( on such distributions; a robust mode estimator is tracked separately (#89). """ values = frame.values + if not bool(np.isfinite(values).all()): + raise ValueError( + "map_estimate is undefined on non-finite draws (NaN or ±inf); strip or " + "impute upstream. Until 2.0.0 an inf draw reached the histogram and " + "crashed with an obscure IndexError from an integer overflow in the bin " + "index — loud, but it named neither the cause nor the caller. This is the " + "same guard exceedance and expected_shortfall carry (register C-57/C-50)" + ) lead = values.shape[:-1] s = values.shape[-1] # Bin in the input dtype, exactly as the v0.2.0 per-row np.histogram did — diff --git a/tests/test_conformance.py b/tests/test_conformance.py index cd752aa..fca0a12 100644 --- a/tests/test_conformance.py +++ b/tests/test_conformance.py @@ -73,7 +73,11 @@ def test_cross_level_alignment_law_is_time_varying(): def test_conformance_floor_is_published(): - assert CONFORMANCE_FLOOR == "1.0.0" + """Moved for the first time in 2.0.0 (ADR-028). + + It stayed 1.0.0 for every release from the freeze until then. + """ + assert CONFORMANCE_FLOOR == "2.0.0" # --- the published frame-envelope checker (ADR-020, C-46) -------------------- diff --git a/tests/test_construction_red.py b/tests/test_construction_red.py index 230f99f..0120da6 100644 --- a/tests/test_construction_red.py +++ b/tests/test_construction_red.py @@ -74,3 +74,39 @@ def test_validate_identifiers_non_1d_raises_value_error(): def test_validate_values_non_array_raises_type_error(): with pytest.raises(TypeError, match="numpy array"): validate_values([[1.0], [2.0]]) # type: ignore[arg-type] + + +# --- construction: `index` must actually be an index ------------------------- +# +# Until 2.0.0 the constructors read exactly one attribute off `index` — `n_rows` — +# and a frame has one, so `PredictionFrame(values, another_frame)` constructed +# silently and the malformed frame then passed `assert_summarizer_contract` +# (falsify audit 2026-08-18; ADR-028). + + +def test_prediction_frame_rejects_a_frame_as_its_index(): + decoy = PredictionFrame(np.zeros((3, 4), dtype=np.float32), _index(3)) + with pytest.raises(TypeError, match="index must be a SpatioTemporalIndex"): + PredictionFrame(np.zeros((3, 4), dtype=np.float32), decoy) + + +def test_feature_frame_rejects_a_frame_as_its_index(): + decoy = PredictionFrame(np.zeros((3, 4), dtype=np.float32), _index(3)) + with pytest.raises(TypeError, match="index must be a SpatioTemporalIndex"): + FeatureFrame(np.zeros((3, 2, 4), dtype=np.float32), decoy, ["a", "b"]) + + +def test_target_frame_rejects_a_frame_as_its_index(): + decoy = PredictionFrame(np.zeros((3, 4), dtype=np.float32), _index(3)) + with pytest.raises(TypeError, match="index must be a SpatioTemporalIndex"): + TargetFrame(np.zeros((3, 1), dtype=np.float32), decoy) + + +def test_construction_rejects_any_object_that_merely_exposes_n_rows(): + """`n_rows` was the whole duck-type, which is why a frame slipped through.""" + + class NotAnIndex: + n_rows = 3 + + with pytest.raises(TypeError, match="got NotAnIndex"): + PredictionFrame(np.zeros((3, 4), dtype=np.float32), NotAnIndex()) diff --git a/tests/test_properties.py b/tests/test_properties.py index 93056d1..522fcff 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -80,3 +80,36 @@ def test_frames_satisfy_runtime_checkable_protocols(protocol): assert isinstance(frame, protocol), ( f"{type(frame).__name__} must satisfy the {protocol.__name__} protocol" ) + + +# --- C-66: the value buffer is write-protected, the caller's array is not ----- + + +@pytest.mark.parametrize("frame", _frames()) +def test_values_buffer_is_read_only(frame): + """ADR-025's enforce, shipped in 2.0.0 (register C-66).""" + assert frame.values.flags.writeable is False + with pytest.raises(ValueError, match="read-only"): + frame.values[...] = 0.0 + + +def test_write_protection_does_not_reach_back_into_the_callers_array(): + """The frame takes a read-only *view*; it must not lock the caller's array. + + `coerce_values` returns the caller's own array when it is already float32 (the + C-07 zero-copy guarantee), so `values.setflags(write=False)` — the literal + one-liner C-66 proposed — would silently make the CALLER's array read-only. + """ + caller = np.ones((4, 3), dtype=np.float32) + frame = PredictionFrame(caller, _index(4)) + assert caller.flags.writeable is True + assert frame.values.flags.writeable is False + caller[0, 0] = 9.0 # the caller still owns their array + + +def test_write_protection_preserves_the_shared_buffer(): + """A view, not a copy — C-07's zero-copy guarantee survives the enforce.""" + caller = np.ones((4, 3), dtype=np.float32) + frame = PredictionFrame(caller, _index(4)) + assert np.shares_memory(frame.values, caller) + assert frame.with_metadata(FrameMetadata()).values.flags.writeable is False diff --git a/tests/test_select.py b/tests/test_select.py index 977d238..2439594 100644 --- a/tests/test_select.py +++ b/tests/test_select.py @@ -129,3 +129,28 @@ def test_reindex_round_trips_with_self(): out = pf.reindex(pf.index) assert np.array_equal(out.values, pf.values) assert np.array_equal(out.identifiers["time"], pf.identifiers["time"]) + + +# --- alignment ops fail loud on a non-index argument ------------------------- +# +# Before 2.0.0 these leaked `AttributeError: '...' object has no attribute +# '_level'` from `SpatioTemporalIndex._require_same_level` — a private attribute +# of a class the caller never named, where ADR-008 requires ValueError/TypeError +# (falsify audit 2026-08-18). + + +@pytest.mark.parametrize("method", ["reindex", "reindex_fill"]) +def test_frame_alignment_rejects_a_non_index_argument(method): + frame = PredictionFrame(np.ones((2, 3), dtype=np.float32), _index([1, 2], [10, 11])) + other = PredictionFrame(np.ones((2, 3), dtype=np.float32), _index([1, 2], [10, 11])) + kwargs = {"fill_value": 0.0} if method == "reindex_fill" else {} + with pytest.raises(TypeError, match="expected a SpatioTemporalIndex"): + getattr(frame, method)(other, **kwargs) + + +def test_index_binary_ops_reject_a_non_index_argument(): + """One guard covers the family — they all route through `_require_same_level`.""" + index = _index([1, 2], [10, 11]) + for op in (index.searchsorted, index.is_superset_of, index.intersect): + with pytest.raises(TypeError, match="expected a SpatioTemporalIndex"): + op("not an index") diff --git a/tests/test_summarize_conformance.py b/tests/test_summarize_conformance.py index 4ad3d8f..66cf09f 100644 --- a/tests/test_summarize_conformance.py +++ b/tests/test_summarize_conformance.py @@ -87,3 +87,22 @@ def test_map_containment_holds_on_tied_integer_draws(): f"{len(failures)}/500 tied-draw posteriors violate the MAP-containment law; " f"first: {failures[0]}" ) + + +# --- the checker must fail on a frame that misreports itself ----------------- + + +def test_conformance_rejects_a_frame_whose_index_is_not_an_index(): + """A published checker must never issue a false pass (docs/CICs/Conformance.md). + + Built through `__new__` on purpose: since 2.0.0 the constructor rejects a + non-index, so this frame cannot be built the ordinary way. That bypass is not + contrived — `with_metadata` uses exactly it, and consumers run this suite + against their own frame factories under ADR-016 (falsify audit 2026-08-18). + """ + malformed = PredictionFrame.__new__(PredictionFrame) + malformed._values = np.zeros((2, 4), dtype=np.float32) + malformed._index = _pf([[1.0, 2.0], [3.0, 4.0]]) # a frame, not an index + malformed._metadata = None + with pytest.raises(AssertionError, match="misreports itself"): + assert_summarizer_contract(malformed) diff --git a/tests/test_summarize_estimators.py b/tests/test_summarize_estimators.py index e51bf0e..4f6b17f 100644 --- a/tests/test_summarize_estimators.py +++ b/tests/test_summarize_estimators.py @@ -3,6 +3,7 @@ from __future__ import annotations import numpy as np +import pytest from views_frames import PredictionFrame, SpatialLevel, SpatioTemporalIndex from views_frames_summarize import hdi, map_estimate, quantiles @@ -80,3 +81,17 @@ def test_quantiles_are_monotonic(): pf = PredictionFrame(rng.random((4, 200), dtype=np.float32), _index(4)) out = quantiles(pf, [0.1, 0.5, 0.9]) assert np.all(np.diff(out, axis=-1) >= 0) + + +# --- C-57: map_estimate fails loud on non-finite draws ----------------------- + + +def test_map_estimate_rejects_non_finite_draws(): + """Shipped with 2.0.0 as a rider on the MAJOR (register C-57). + + Before it, an inf draw overflowed the integer bin index and crashed with a bare + `IndexError` naming neither the cause nor the caller. + """ + for bad in (np.inf, -np.inf, np.nan): + with pytest.raises(ValueError, match="non-finite draws"): + map_estimate(_pf([[0.0, 1.0, bad]])) diff --git a/uv.lock b/uv.lock index f8298b5..89a4605 100644 --- a/uv.lock +++ b/uv.lock @@ -3438,7 +3438,7 @@ wheels = [ [[package]] name = "views-frames" -version = "1.11.0" +version = "2.0.0" source = { editable = "." } dependencies = [ { name = "numpy", version = "2.2.6", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.11'" },