2.0.0 — a frame's index must actually be an index (the first MAJOR) - #266
Merged
Merged
Conversation
… buffer BREAKING CHANGE: the three frame constructors now raise TypeError when `index` is not a SpatioTemporalIndex, `frame.values` is read-only, alignment ops raise TypeError on a non-index argument, and map_estimate raises on non-finite draws. The code half of 2.0.0. Documents and the release follow. **The index was never type-checked.** Construction read exactly one attribute off `index` — `n_rows` — and a frame has one, so `PredictionFrame(values, another_frame)` constructed silently, as did any object exposing `n_rows`. `y_pred` got four guards; the argument the package is named for got none. Found by a falsification audit of the claim that this repo was finished. It was not merely untidy. `assert_summarizer_contract` reads only `values` and `n_rows`, never `index`, so it **certified** such a frame — a published checker issuing a false pass, which docs/CICs/Conformance.md names as the failure mode this module must never have. The checker keeps its own assertion even though construction now makes it unreachable by ordinary means: consumers run the suite against their own factories under ADR-016, and `with_metadata` already builds frames through `__new__`. Guard written three times rather than extracted. `_validation.py` is imported BY `index.py`, so it cannot import SpatioTemporalIndex back without a cycle, and ADR-011 Option C already accepts WET across the three siblings as the price of no shared base. **Alignment ops leaked a private attribute.** `_require_same_level` read `other._level` unchecked, so reindex/reindex_fill/is_superset_of/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. Every same-level binary op routes through that one helper, so one guard covers the family. **C-66 rides this MAJOR, and not as the one-liner it was written as.** ADR-025 records 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 it would have silently made the CALLER's array read-only, action at a distance well outside this contract. Measured, not 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 writeable, and still shares memory. Verified the memmap path keeps its subclass and zero-copy through `.view()`. Noted, not changed: SpatioTemporalIndex has this same aliasing effect on the identifier arrays and its comment calls them "views", which `ascontiguousarray` does not return for already-contiguous input. Comment corrected to say what the code does; behaviour left alone as out of scope. **C-57 rides too** — map_estimate now carries the same np.isfinite guard exceedance and expected_shortfall have. An inf draw used to overflow the integer bin index and crash with a bare IndexError naming neither cause nor caller. Every guard has a test; coverage stays 100% line and branch, so no branch is asserted by inspection. mypy passes on 3.10 and 3.11 (the `.view()` return needed an explicit annotation — numpy's stubs type it Any, which silently widened the values property). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…or 2.0.0 The governance half of 2.0.0. GOVERNANCE's MAJOR process asks for the decision to be proposed as an ADR and the floor bumped; both are here. **ADR-028** records why the first MAJOR exists, what breaks, and the migration. The migration table is the part a consumer needs: five rows, four of which say "unchanged". The only row where working code could plausibly stop working is an in-place `.values` mutation, and ADR-025 already documented that as unsupported. No `from_legacy_*` shim, and the ADR says why rather than leaving its absence to be noticed: no wire format, layout or signature changes — only the set of inputs that were never valid narrows, and a shim would have nothing to translate. **CONFORMANCE_FLOOR 1.0.0 -> 2.0.0** — the first move since the freeze. It moved because `assert_summarizer_contract` now rejects a frame that misreports its own index, and GOVERNANCE bumps the floor on any breaking change to any published entry point. Every consumer's CI will assert a new contract version; that is what the constant is for, but it has never happened before, so it is stated in the ADR, GOVERNANCE, the CIC and the code rather than left to be discovered. **ADR-018 and ADR-025 amended.** ADR-018 because the freeze it declares has now been broken once, deliberately, and a reader must not find that out from the CHANGELOG. ADR-025 because its title says immutability is "by convention" and that is now history — and because the one-line fix it recorded would have been wrong, which is worth writing down where the next person will look. **Register: 92 entries, 10 open, 82 resolved. Every open entry waits on an event outside this repository.** C-66 and C-57 resolved as riders. C-13 stays open as the standing coordinated-bump condition, now carrying an execution record of what this MAJOR actually did — the checklist earned its keep by forcing C-43 to be declined explicitly rather than forgotten, and by turning "consumers will pick it up" into three named lockfiles. **C-43 declined, with its precondition rewritten.** "#89 or a MAJOR" silently became false the moment 2.0.0 tagged, so it now reads "#89". 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. Extracting a shared helper changes the output of one of them for a Tier-4 entry with no correctness impact. **Three new entries.** C-93 (the index type gap and the checker's false pass, Tier 2 — a false pass in a published contract checker is worse than no checker, because it is evidence a consumer is entitled to rely on) and C-94 (the private attribute leak, Tier 3) are resolved on arrival. C-95 is new and open: the index still write-protects the caller's identifier arrays in place, the hazard C-66 had to avoid for the frames. Left alone on purpose — it is ADR-025's own reasoning applied consistently rather than an exception made because the MAJOR was already open. Its comment, which claimed "views" that ascontiguousarray does not return, is corrected. Caught while verifying rather than after: C-95 was first written into the Resolved section, where the clause-4 check's awk range cannot see it. It would have passed by being hidden rather than by being external. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Version, CHANGELOG, and every claim elsewhere that 2.0.0 makes false. The CHANGELOG leads with the migration rather than the reasoning, because what a consumer needs is one constraint change and a table of five rows, four of which say "unchanged". The reasoning lives in ADR-028 for whoever wants it. Corrected here because 2.0.0 falsifies them, not as housekeeping: - README design principle 2 and §immutability said the value buffer is "immutable by convention" and "left writeable on purpose". Both were true until this release. - CLAUDE.md said the same, and that CONFORMANCE_FLOOR stays 1.0.0. It has moved. - CLAUDE.md's maintenance-mode section said the API "has been frozen since v1.0.0". It was frozen at v1.0.0 and has now been broken once. The section stays — this release is not a licence to reopen the repo, and the honest version of that sentence says so more clearly than the old one did. The README chronicle gains 2.0.0 with what actually changed and the one-line migration, so a reader who never opens the CHANGELOG still learns they need to re-lock and nothing more. Not yet tagged. C-13's pre-tag checklist requires an adoption issue filed in each of the three pinned consumer repos first — views-faoapi, views-postprocessing, views-crafdapi — and this repo cannot open issues in sibling repos. That gate is the maintainer's; merging is not blocked by it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the two real defects from the 2026-08-18 falsification audit, and carries the freeze-cluster riders C-13 has been holding for a MAJOR.
The defect
The three constructors validated
y_predfour times and read exactly one attribute offindex—n_rows. A frame has one. So this constructed silently:Every summarizer reads
.valuesand never.index, so the malformed frame returned numerically correct answers — andassert_summarizer_contractcertified it. That checker is published under ADR-016 and consumers run it in their own CI, anddocs/CICs/Conformance.mdnames "a checker that cannot detect a violation" as the failure mode it must never have.Why MAJOR
GOVERNANCE.mdlists "tightening an invariant" as MAJOR; ADR-018 freezes the constructor shapes. The test this repo applies is "does any currently-succeeding call start raising?" — C-57 answers no (already crashed, just uglily) and rides free; the index guard answers yes. ADR-025/C-66 is the exact precedent and went the same way.Riders (register C-13)
frame.valueswrite-protected. Not as the one-liner ADR-025 recorded:coerce_valuesreturns the caller’s own array when it is already float32, sosetflags(write=False)would silently make the caller’s array read-only. Measured — it does. Ships as a read-only view: frame buffer locked, caller untouched, memory still shared, memmap subclass preserved.map_estimaterejects non-finite draws.Consumer impact
Change the constraint, re-lock, carry on. Five behaviours change; four rows of the migration table say "unchanged".
CONFORMANCE_FLOORmoves1.0.0→2.0.0— its first move ever.Verification
Register: 92 entries, 10 open, 82 resolved — every open entry waits on an event outside this repo.
Not tagged yet
C-13 requires an adoption issue in each of views-faoapi, views-postprocessing and views-crafdapi before tagging. I cannot open issues in sibling repos; text is drafted and waiting.
🤖 Generated with Claude Code