diff --git a/CHANGELOG.md b/CHANGELOG.md index eb34896..b1ea123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,26 @@ 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`. +## [1.10.1] — 2026-07-28 + +**`io.arrow.load` now validates the wire-contract row order before reshaping (#199 item 1).** +Fail-loud hardening of the parquet load path; no API change; `CONFORMANCE_FLOOR` stays `1.0.0`. + +### Fixed +- **Silent sample-slot corruption on out-of-order parquet input.** `arrow.save` writes the + row order as the contract (`sample = tile(arange(S), N)`; `io/arrow.py`) but `load` + reconstructed positionally without ever checking it — a reordered, truncated, or + foreign-rewritten table reshaped **plausible floats into the wrong sample slots with no + error**. `load` now validates the layout first and raises `ValueError` on: a row count + that is not a positive multiple of the header's `n_samples` (truncated/filtered table); + a `sample` column deviating from the written tile order (row-level reorder); or + `time`/`unit` not constant within a sample block (rows swapped between cells — the case + the tile check alone cannot see). A whole-cell block reorder (identifiers travel with + their draws) remains a *consistent* table and still loads. Implements the check + views-postprocessing ADR-013 §4.5(b) previously required every consumer to run + themselves on a separate raw-table read — the leaf now hardens all consumers at once. + Register C-72. (#199 item 2 — mmap/partitioned arrow reading — remains open.) + ## [1.10.0] — 2026-07-27 **The dense-grid fill primitive (ADR-026) — unblocks pandas-free FAO ingestion (#203).** diff --git a/pyproject.toml b/pyproject.toml index 492c8cd..b9cf2ca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "views-frames" -version = "1.10.0" +version = "1.10.1" 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 692fe3c..93be52e 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -4,10 +4,10 @@ |-------------------|--------------------------------------| | Project | views-frames | | Owner | VIEWS platform maintainers | -| Last Updated | 2026-07-27 | -| Total Concerns | 68 | +| Last Updated | 2026-07-28 | +| Total Concerns | 69 | | Open Concerns | 13 | -| Resolved Concerns | 55 | +| Resolved Concerns | 56 | | Disagreements | 12 | --- @@ -394,6 +394,20 @@ Cross-refs: C-47 (eval provenance kept out of the generic header — the precede ## Resolved Concerns +### C-72: `arrow.load` trusted the parquet row order it never checked — silent sample-slot corruption on out-of-order input — RESOLVED + +| Field | Value | +|-------|-------| +| ID | C-72 | +| Tier | 2 (silent data corruption, but only reachable via an out-of-contract intermediary mutating the file between `save` and `load` — the supported save→load path was always correct) | +| Source | views-postprocessing ADR-013 §4.5(b) (the consumer-side check it mandated); filed as #199 item 1 (2026-07-19); fixed 2026-07-28 (v1.10.1). | +| Resolved | 2026-07-28 (v1.10.1, #199 item 1) | +| Location | `src/views_frames/io/arrow.py` (`load` — the positional `reshape(n, s)` reconstruction). | +| Resolution | `load` now validates the wire-contract layout before reshaping and raises `ValueError` on: row count not a positive multiple of the header's `n_samples` (truncated/filtered); the `sample` column deviating from the written `tile(arange(S), N)` order (row-level reorder); or `time`/`unit` not constant within a sample block (cross-cell row swaps — invisible to the tile check alone). A whole-cell block reorder stays loadable (identifiers travel with their draws — a consistent table). Red tests pin all three raises + the consistent-reorder boundary (`tests/test_io.py`). Consumers' WET pre-checks per ADR-013 §4.5(b) are now redundant — the leaf hardens every consumer at once. Residual: #199 item 2 (mmap/partitioned arrow reading) stays open in the issue — an optimization, explicitly not a contract dependency. | +| Cross-refs | C-29 (the original io red-team family this extends), C-51 (assert-raise-path testing convention), #199. | + +--- + ### C-70: audit polish bundle — docs narrative epoch-lag + three small test adds (four-axis audit 2026-07) — RESOLVED | Field | Value | diff --git a/src/views_frames/io/arrow.py b/src/views_frames/io/arrow.py index 18968fa..2572924 100644 --- a/src/views_frames/io/arrow.py +++ b/src/views_frames/io/arrow.py @@ -71,7 +71,20 @@ def save( def load(path: Path | str) -> dict[str, Any]: - """Read a flat-columnar parquet frame state written by :func:`save`.""" + """Read a flat-columnar parquet frame state written by :func:`save`. + + The reconstruction is positional (``reshape(n, s)``), so the table's row + order **is** the contract: before trusting it, the wire-contract layout is + validated and any violation **raises** rather than reshaping plausible + floats into the wrong sample slots (ADR-013 §4.5b wire contract; #199). + + Raises: + ValueError: the table is truncated/filtered (row count not a multiple + of the header's ``n_samples``), the ``sample`` column deviates from + the written ``tile(arange(S), N)`` order, or ``time``/``unit`` are + not constant within a row's sample block (rows swapped between + cells) — a reordered, truncated, or foreign-rewritten table. + """ table = pq.read_table(str(path)) raw = table.schema.metadata or {} header = json.loads(raw[b"views_frames"].decode()) @@ -80,9 +93,39 @@ def load(path: Path | str) -> dict[str, Any]: time_col = table.column("time").to_numpy() unit_col = table.column("unit").to_numpy() - n = time_col.shape[0] // s - time = time_col.reshape(n, s)[:, 0] - unit = unit_col.reshape(n, s)[:, 0] + total = int(time_col.shape[0]) + if s <= 0 or total % s != 0: + raise ValueError( + f"corrupt frame parquet: {total} rows is not a positive multiple of " + f"the header's n_samples={s} — a truncated or filtered table cannot " + "be reshaped safely (wire contract, #199)" + ) + n = total // s + + sample_col = table.column("sample").to_numpy() + if not np.array_equal(sample_col, np.tile(np.arange(s, dtype=np.int32), n)): + raise ValueError( + "corrupt frame parquet: the 'sample' column deviates from the " + "written tile(arange(S), N) order — the table was reordered or " + "rewritten, so a positional reshape would silently place draws in " + "the wrong sample slots (wire contract, #199)" + ) + + time_blocks = time_col.reshape(n, s) + unit_blocks = unit_col.reshape(n, s) + if not ( + bool((time_blocks == time_blocks[:, :1]).all()) + and bool((unit_blocks == unit_blocks[:, :1]).all()) + ): + raise ValueError( + "corrupt frame parquet: 'time'/'unit' are not constant within a " + "row's sample block — rows were swapped between cells, so a " + "positional reshape would silently misattribute draws " + "(wire contract, #199)" + ) + + time = time_blocks[:, 0] + unit = unit_blocks[:, 0] if ndim == 2: values = table.column("value").to_numpy().reshape(n, s).astype(np.float32) diff --git a/tests/test_io.py b/tests/test_io.py index 9dcdf93..0410388 100644 --- a/tests/test_io.py +++ b/tests/test_io.py @@ -173,3 +173,66 @@ def test_arrow_load_non_frame_parquet_raises(tmp_path): pq.write_table(pa.table({"x": [1, 2, 3]}), str(path)) with pytest.raises(KeyError): arrow.load(path) + + +# --- G2 RED: wire-contract ordering validation on load (#199, ADR-013 §4.5b) -- +# 🟥 Red team: `load` reshapes positionally, so the row order IS the contract. +# A reordered / truncated / cross-swapped table previously reshaped plausible +# floats into the wrong sample slots silently; every such violation must raise. + + +def _saved_table(tmp_path): + import pyarrow.parquet as pq + + st = _state_2d() + path = tmp_path / "frame.parquet" + arrow.save(path, **st) + return path, pq.read_table(str(path)) + + +def test_arrow_load_reordered_rows_raise(tmp_path): + # Reversing the rows breaks the written tile(arange(S), N) sample order. + import pyarrow.parquet as pq + + path, table = _saved_table(tmp_path) + pq.write_table(table.take(list(range(table.num_rows - 1, -1, -1))), str(path)) + with pytest.raises(ValueError, match="reordered"): + arrow.load(path) + + +def test_arrow_load_truncated_table_raises(tmp_path): + # Dropping a row makes the row count a non-multiple of n_samples. + import pyarrow.parquet as pq + + path, table = _saved_table(tmp_path) + pq.write_table(table.slice(0, table.num_rows - 1), str(path)) + with pytest.raises(ValueError, match="truncated or filtered"): + arrow.load(path) + + +def test_arrow_load_cross_cell_row_swap_raises(tmp_path): + # Swap the sample-0 rows of two different (time, unit) cells: the sample + # column still reads tile(arange(S), N), so the tile check alone passes — + # only the block-constancy check catches the identifier/value misalignment. + import pyarrow.parquet as pq + + path, table = _saved_table(tmp_path) + order = list(range(table.num_rows)) + order[0], order[2] = order[2], order[0] # (1,10,s0) <-> (1,11,s0) + pq.write_table(table.take(order), str(path)) + with pytest.raises(ValueError, match="not constant within"): + arrow.load(path) + + +def test_arrow_load_whole_cell_reorder_still_loads(tmp_path): + # 🟩 Boundary of the contract: moving a WHOLE cell block (identifiers travel + # with their draws) is a consistent table — a reordered-but-valid frame + # loads, with the identifiers following the moved values. + import pyarrow.parquet as pq + + path, table = _saved_table(tmp_path) + order = [2, 3, 0, 1, 4, 5] # swap the first two complete (time, unit) blocks + pq.write_table(table.take(order), str(path)) + out = arrow.load(path) + assert np.array_equal(out["unit"], np.array([11, 10, 10])) + assert np.array_equal(out["values"][0], np.array([2.0, 3.0], dtype=np.float32)) diff --git a/uv.lock b/uv.lock index bd62752..ae2a897 100644 --- a/uv.lock +++ b/uv.lock @@ -3274,7 +3274,7 @@ wheels = [ [[package]] name = "views-frames" -version = "1.10.0" +version = "1.10.1" source = { editable = "." } dependencies = [ { name = "numpy", version = "2.2.6", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.11'" },