From 99fd57ec6d5f3815d6b242af73022bc4f8c5486e Mon Sep 17 00:00:00 2001 From: Polichinl Date: Wed, 2 Sep 2026 23:21:53 +0200 Subject: [PATCH] docs: contracts stop describing behaviour that changed under them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2b of the governance consistency review — the class where a change landed, sibling documents were updated, and the document describing the behaviour was not. 5 findings, 3 of the review's 9 contradictions. **docs/CICs/FrameMetadata.md was wrong about the code it cites.** Its §6 table said a non-JSON metadata field is "**Silently stringified** — `io/npz.py` uses `json.dumps(..., default=str)`" while arrow raises. Commit 5423431 deleted that `default=str` on 2026-08-18 to close register C-90, three hours after this CIC was written. A consumer reading §6 today would build a workaround for a bug that has not existed since August, guided by a quotation of a line that no longer exists. **Its §10 said three guarantees are "not pinned by anything".** The same commit added all three tests. The commit stat shows why it was missed: 5423431 updated PredictionFrame.md, Summarize.md and TargetFrame.md — and not FrameMetadata.md. The CIC that described the gaps was the one document not updated when they closed. Both are mine, from the same day, and together they are the fifth recorded instance of the pattern C-96 names: a fix that reaches the instance and not the class. The §10 items are rewritten to name their closing tests rather than deleted. A CIC states its guarantees whether or not a test exists (ADR-006), and this document is the reason the gaps were found at all — written, gaps noticed while writing, gaps closed the same day. `docs/validate_docs.sh` check 12 now verifies 21 test references instead of 19, so the three newly-named tests are themselves guarded against renaming. **docs/CICs/Reconcile.md** told the reader that writing to `.values` "may corrupt shares". Since 2.0.0 it raises `ValueError` (ADR-028 / C-66) — the corruption it warns about is no longer reachable. **README.md** said `scripts/` holds "standalone dev tools, none wired into CI". Two of six run in the `docs` job. I made that false on 2026-08-18 by wiring in `check_arch_tree.py` and more false this afternoon by adding `check_doc_refs.py`. The line sits inside README's directory tree, which `check_arch_tree.py` validates — but that check reads module paths and ignores the prose beside them. **research/coherent_posterior_summary_note.md** asserted `tip_mass` default `0.5`. It has been `0.25` since ADR-019 Amendment 3 (2026-07-24). The note carries a v1.3.0 stamp, so the value was right when written; it now says both. Also corrected: a `tests/test_select.py:88` citation, which the register's own convention forbids — "name things, never line numbers", because line numbers drift silently. It now names `test_select_preserves_metadata`. **One finding did not survive verification and is not fixed.** F-B8-03 claimed `research/figures/NOTE.md` predates Amendment 4's correction to the MAP-containment law. It does not: the note says "every floor holding more than half the tip floor's **draws**", which is the corrected formulation and matches the shipped `2 * n_floor > n_tip`. The pre-correction version counted index spans. The note was already right. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- docs/CICs/FrameMetadata.md | 46 ++++++++++----------- docs/CICs/Reconcile.md | 6 +-- research/coherent_posterior_summary_note.md | 3 +- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 8db632f..d2988ef 100644 --- a/README.md +++ b/README.md @@ -445,7 +445,7 @@ views-frames/ │ ├── module.py # ReconciliationModule (holds the injected mapping) │ ├── result.py # ReconciliationResult — the frame plus HOW it was made │ └── conformance.py # assert_reconcile_contract -├── scripts/ # standalone dev tools, none wired into CI +├── scripts/ # dev tools; check_arch_tree + check_doc_refs run in CI ├── examples/ # runnable quickstarts (run by CI) └── tests/ # flat: test_*.py + fixtures/ ``` diff --git a/docs/CICs/FrameMetadata.md b/docs/CICs/FrameMetadata.md index 75d658b..d7be314 100644 --- a/docs/CICs/FrameMetadata.md +++ b/docs/CICs/FrameMetadata.md @@ -97,8 +97,7 @@ every field in it is a field every consumer must agree on. | Situation | Behaviour | Loud? | |---|---|---| | Unknown key in `from_dict` | **Dropped silently** | ❌ **no** | -| Field set to a non-JSON type, then `save`d via **npz** | **Silently stringified** — `io/npz.py` uses `json.dumps(..., default=str)`, so a `datetime` timestamp reloads as `'2026-01-01 00:00:00'` | ❌ **no** | -| The same value `save`d via **arrow** | `TypeError: Object of type datetime is not JSON serializable` — `io/arrow.py` uses a plain `json.dumps` | ✅ yes | +| Field set to a non-JSON type, then `save`d via **either codec** | `TypeError: Object of type datetime is not JSON serializable` — both `io/npz.py` and `io/arrow.py` use a plain `json.dumps` | ✅ yes | | Field set to the wrong type at a typed call site | Accepted at runtime | ❌ no — caught by `mypy --strict` **only here** | | Wrong type arriving from a loaded `header.json` | Accepted; `from_dict` takes `Mapping[str, Any]` and coerces nothing | ❌ **no** — mypy cannot see this path | | Mutating a field after construction | `FrozenInstanceError` | ✅ yes | @@ -177,33 +176,34 @@ level = frame.metadata.run_type.split("_")[0] # ADR-003: no semantic inference - **Green:** `to_dict`/`from_dict` round-trip preserves set fields (`test_metadata_to_from_dict_roundtrip`, `test_metadata_provenance_roundtrip`); unset fields are omitted from `to_dict` (`test_metadata_generic_provenance_fields_default_none`) — all in - `tests/test_frames.py`. Metadata survives row selection (`tests/test_select.py:88`). + `tests/test_frames.py`. Metadata survives row selection (`tests/test_select.py::test_select_preserves_metadata`). - **Beige:** `with_metadata` allocates no second `values` buffer — the copy-vs-view property (`tests/test_properties.py::test_with_metadata_shares_the_values_buffer`, register C-07). - **Red:** mutation raises `FrozenInstanceError` (`test_metadata_is_frozen`). `test_metadata_ignores_unknown_keys` is **green, not red, and pins less than its name suggests**: it asserts only `md.model == "x"` after passing an unknown key, so it pins *"does not raise"* and -never asserts the unknown key was discarded rather than stored. The drop documented in §3 and §6 -— the behaviour with the durable consequence — is therefore **not pinned by anything**. - -**Three guarantees in §3 are not pinned by any test**, found while writing this contract and -recorded rather than glossed: - -0. **The unknown-key drop itself** — see immediately above. `assert not hasattr(md, "unknown")` - is the missing line. - -1. **The `save`/`load` header round-trip is pinned for `PredictionFrame` only** - (`tests/test_frames.py:107`, `assert loaded.metadata == pf.metadata`). `FeatureFrame` and - `TargetFrame` have no equivalent assertion — `test_feature_frame_save_load_preserves_names` - covers `feature_names`, not the header. -2. **Nothing asserts that a frame built without metadata exposes an empty header** rather than - `None`, though §3 guarantees it and every consumer reading `.metadata` depends on it. - -Both are one-line additions to an existing test file. They belong to **S9 (#249)**, which owns -the test suite's self-description under register C-80; noted there. This contract states the -guarantee regardless — that is what a CIC is for (ADR-006: tests are derived *from* the -contract), and naming the gap is better than a §10 that reads complete. +never asserts the unknown key was discarded rather than stored. + +**Three guarantees in §3 were unpinned when this contract was written, and all three were pinned +the same day** (2026-08-18, register C-80, commit `5423431`). They are listed here because the +gap is worth remembering, not because it is open — each now names the test that closes it: + +0. **The unknown-key drop itself** — that an unknown key is *discarded*, not merely tolerated. + Now pinned by `tests/test_frames.py::test_metadata_unknown_keys_are_dropped_not_stored`. + +1. **The `save`/`load` header round-trip was pinned for `PredictionFrame` only** + (`tests/test_frames.py::test_metadata_survives_save_load_for_all_three_frames` now covers all + three; `test_feature_frame_save_load_preserves_names` covers `feature_names`, not the header). + +2. **Nothing asserted that a frame built without metadata exposes an empty header** rather than + `None`, though §3 guarantees it and every consumer reading `.metadata` depends on it. Now + pinned by `tests/test_frames.py::test_frame_without_metadata_exposes_an_empty_header_not_none`. + +All three were closed by register **C-80**. The gap is recorded rather than deleted because a CIC +states its guarantees whether or not a test exists (ADR-006: tests are derived *from* the +contract), and because this document is the reason the gap was found at all — it was written, +the gaps were noticed while writing it, and they were closed the same day. --- diff --git a/docs/CICs/Reconcile.md b/docs/CICs/Reconcile.md index 9035463..d95d0d2 100644 --- a/docs/CICs/Reconcile.md +++ b/docs/CICs/Reconcile.md @@ -228,9 +228,9 @@ ReconciliationModule().reconcile(cm, pgm) # TypeError — map_keys/map # approximation (C-62 / ADR-024) — read res.mode and treat joint tails as uncalibrated. res = module.reconcile_result(cm, pgm) # res.mode == ALIGNED_DRAWS is a caveat -# WRONG: mutating the returned frame's value buffer in place (the leaf value buffer is -# immutable-by-convention; see ADR-025 / PredictionFrame CIC §9) -> may corrupt shares -out.values[:] = 0 # unsupported +# WRONG: mutating the returned frame's value buffer in place. Since 2.0.0 the buffer is +# write-protected (ADR-028 / register C-66), so this raises rather than corrupting shares. +out.values[:] = 0 # ValueError: assignment destination is read-only # WRONG: stamping the mode onto the leaf frame's metadata. It is reported on the result. out.with_metadata(mode="point-broadcast") # the leaf carries no reconcile vocabulary diff --git a/research/coherent_posterior_summary_note.md b/research/coherent_posterior_summary_note.md index 1c52249..80ae8b6 100644 --- a/research/coherent_posterior_summary_note.md +++ b/research/coherent_posterior_summary_note.md @@ -101,7 +101,8 @@ A `k <= 0` floor (too few samples to hold two draws, at small `S`) collapses to ### 3.3 A mass-aware tip — the *shorth* (robust mode, distribution-agnostic zero handling) -The point estimate is the **median of the floor at a configurable `tip_mass`** (default `0.5` — +The point estimate is the **median of the floor at a configurable `tip_mass`** (default `0.25` +since ADR-019 Amendment 3, 2026-07-24; this note was written at v1.3.0 when it was `0.5` — the *shorth*, the shortest-half mode), **not** the degenerate narrowest floor. Two properties follow: