Skip to content

Commit fba975e

Browse files
d-v-bclaude
andcommitted
test: codec invariants + pipeline parity matrix
Two new test files: test_codec_invariants — asserts contract-level properties that every codec / shard / buffer combination must satisfy: round-trip exactness, prototype propagation, fill-value handling, all-empty shard handling. test_pipeline_parity — exhaustive matrix asserting that SyncCodecPipeline and BatchedCodecPipeline produce semantically identical results across codec configs, layouts (including nested sharding), write sequences, and write_empty_chunks settings. Three checks per cell: 1. Same array contents on read. 2. Same set of store keys after writes. 3. Each pipeline reads the other's output identically (catches layout-divergence bugs). These tests pinned the design throughout the SyncCodecPipeline + partial-shard development. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a48f4f7 commit fba975e

2 files changed

Lines changed: 705 additions & 0 deletions

File tree

tests/test_codec_invariants.py

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
"""Codec / shard / buffer invariants.
2+
3+
These tests enforce the contracts described in
4+
``docs/superpowers/specs/2026-04-17-codec-pipeline-invariants.md``.
5+
They exist to catch the class of bug where pipeline code reasons
6+
case-by-case about how codecs, shards, IO, and buffers interact and
7+
silently breaks a combination.
8+
9+
Each test is short and focused on one invariant. If any test here
10+
fails, the corresponding section of the design doc points at what
11+
contract was broken.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
from dataclasses import replace
17+
from typing import TYPE_CHECKING, Any
18+
from unittest.mock import patch
19+
20+
import numpy as np
21+
import pytest
22+
23+
if TYPE_CHECKING:
24+
from pathlib import Path
25+
26+
import zarr
27+
from zarr.abc.codec import BytesBytesCodec, Codec
28+
from zarr.abc.store import SupportsSetRange
29+
from zarr.codecs.bytes import BytesCodec
30+
from zarr.codecs.crc32c_ import Crc32cCodec
31+
from zarr.codecs.gzip import GzipCodec
32+
from zarr.codecs.transpose import TransposeCodec
33+
from zarr.codecs.zstd import ZstdCodec
34+
from zarr.core.array_spec import ArrayConfig, ArraySpec
35+
from zarr.core.buffer import Buffer, default_buffer_prototype
36+
from zarr.core.codec_pipeline import ChunkTransform, SyncCodecPipeline
37+
from zarr.core.dtype import get_data_type_from_native_dtype
38+
from zarr.storage import LocalStore, MemoryStore
39+
40+
# ---------------------------------------------------------------------------
41+
# Helpers
42+
# ---------------------------------------------------------------------------
43+
44+
45+
def _spec(
46+
shape: tuple[int, ...] = (10,),
47+
dtype: str = "float64",
48+
*,
49+
fill_value: object = 0.0,
50+
write_empty_chunks: bool = False,
51+
) -> ArraySpec:
52+
zdtype = get_data_type_from_native_dtype(np.dtype(dtype))
53+
return ArraySpec(
54+
shape=shape,
55+
dtype=zdtype,
56+
fill_value=zdtype.cast_scalar(fill_value),
57+
config=ArrayConfig(order="C", write_empty_chunks=write_empty_chunks),
58+
prototype=default_buffer_prototype(),
59+
)
60+
61+
62+
# ---------------------------------------------------------------------------
63+
# C1: Codecs only mutate `shape`
64+
# ---------------------------------------------------------------------------
65+
66+
# Codecs that we expect to satisfy C1 unconditionally. Each is in a
67+
# state where calling resolve_metadata is safe with the helper spec.
68+
_C1_CODECS: list[Codec] = [
69+
BytesCodec(),
70+
Crc32cCodec(),
71+
GzipCodec(level=1),
72+
ZstdCodec(level=1),
73+
TransposeCodec(order=(0,)),
74+
]
75+
76+
77+
@pytest.mark.parametrize("codec", _C1_CODECS, ids=lambda c: type(c).__name__)
78+
def test_C1_resolve_metadata_only_mutates_shape(codec: Codec) -> None:
79+
"""C1: prototype, dtype, fill_value, config never change across the codec chain."""
80+
spec_in = _spec()
81+
spec_out = codec.resolve_metadata(spec_in)
82+
assert spec_out.prototype is spec_in.prototype, f"{type(codec).__name__} changed prototype"
83+
assert spec_out.dtype == spec_in.dtype, f"{type(codec).__name__} changed dtype"
84+
assert spec_out.fill_value == spec_in.fill_value, f"{type(codec).__name__} changed fill_value"
85+
assert spec_out.config == spec_in.config, f"{type(codec).__name__} changed config"
86+
87+
88+
# ---------------------------------------------------------------------------
89+
# C2: Each codec call receives the runtime chunk_spec
90+
# ---------------------------------------------------------------------------
91+
92+
93+
class _PrototypeRecordingCodec(BytesBytesCodec): # type: ignore[misc,unused-ignore]
94+
"""A no-op BB codec that records the prototype it was called with."""
95+
96+
is_fixed_size = True
97+
seen_prototypes: list[object]
98+
99+
def __init__(self) -> None:
100+
object.__setattr__(self, "seen_prototypes", [])
101+
102+
def to_dict(self) -> dict[str, Any]:
103+
return {"name": "_prototype_recording", "configuration": {}}
104+
105+
@classmethod
106+
def from_dict(cls, data: dict[str, Any]) -> _PrototypeRecordingCodec:
107+
return cls()
108+
109+
def compute_encoded_size(self, input_byte_length: int, _spec: ArraySpec) -> int:
110+
return input_byte_length
111+
112+
def _decode_sync(self, chunk_bytes: Buffer, chunk_spec: ArraySpec) -> Buffer:
113+
self.seen_prototypes.append(chunk_spec.prototype)
114+
return chunk_bytes
115+
116+
def _encode_sync(self, chunk_bytes: Buffer, chunk_spec: ArraySpec) -> Buffer | None:
117+
self.seen_prototypes.append(chunk_spec.prototype)
118+
return chunk_bytes
119+
120+
async def _decode_single(self, chunk_bytes: Buffer, chunk_spec: ArraySpec) -> Buffer:
121+
return self._decode_sync(chunk_bytes, chunk_spec)
122+
123+
async def _encode_single(self, chunk_bytes: Buffer, chunk_spec: ArraySpec) -> Buffer | None:
124+
return self._encode_sync(chunk_bytes, chunk_spec)
125+
126+
127+
def test_C2_chunk_transform_uses_runtime_prototype() -> None:
128+
"""C2: the prototype the codec sees comes from the runtime chunk_spec, not a cache."""
129+
from zarr.core.buffer import BufferPrototype
130+
131+
recording = _PrototypeRecordingCodec()
132+
transform = ChunkTransform(codecs=(BytesCodec(), recording))
133+
134+
proto_default = default_buffer_prototype()
135+
# A distinct BufferPrototype instance with the same buffer/nd_buffer
136+
# types — fails identity check but works at runtime.
137+
proto_other = BufferPrototype(buffer=proto_default.buffer, nd_buffer=proto_default.nd_buffer)
138+
assert proto_other is not proto_default
139+
140+
spec_a = replace(_spec(), prototype=proto_default)
141+
spec_b = replace(_spec(), prototype=proto_other)
142+
143+
arr = proto_default.nd_buffer.from_numpy_array(np.arange(10, dtype="float64"))
144+
transform.encode_chunk(arr, spec_a)
145+
transform.encode_chunk(arr, spec_b)
146+
147+
assert recording.seen_prototypes[0] is proto_default
148+
assert recording.seen_prototypes[1] is proto_other, (
149+
"ChunkTransform did not pass the runtime prototype to the codec"
150+
)
151+
152+
153+
# ---------------------------------------------------------------------------
154+
# C3: pipeline never branches on codec type
155+
# ---------------------------------------------------------------------------
156+
157+
158+
def test_C3_pipeline_methods_do_not_isinstance_check_sharding_codec() -> None:
159+
"""C3: Pipeline read/write methods must use supports_partial_*, not isinstance(ShardingCodec).
160+
161+
Static check: scan the pipeline classes' read/write methods for
162+
`isinstance(..., ShardingCodec)`. Other helpers (e.g. metadata
163+
validation in `codecs_from_list`) may legitimately need the check.
164+
"""
165+
import inspect
166+
import re
167+
168+
from zarr.core.codec_pipeline import BatchedCodecPipeline, SyncCodecPipeline
169+
170+
pattern = re.compile(r"isinstance\s*\([^)]*ShardingCodec[^)]*\)")
171+
172+
for cls in (SyncCodecPipeline, BatchedCodecPipeline):
173+
for method_name in ("read", "write", "read_sync", "write_sync"):
174+
method = getattr(cls, method_name, None)
175+
if method is None:
176+
continue
177+
source = inspect.getsource(method)
178+
matches = pattern.findall(source)
179+
assert not matches, (
180+
f"{cls.__name__}.{method_name} contains isinstance check on "
181+
f"ShardingCodec; use supports_partial_encode/decode instead. "
182+
f"Matches: {matches}"
183+
)
184+
185+
186+
# ---------------------------------------------------------------------------
187+
# S1 + S2: shard layout is compact and skips empty chunks by default
188+
# ---------------------------------------------------------------------------
189+
190+
191+
def test_S2_empty_chunks_omitted_under_default_config() -> None:
192+
"""S2: writing fill-value data must not produce store keys for those chunks."""
193+
store = MemoryStore()
194+
arr = zarr.create_array(
195+
store=store,
196+
shape=(20,),
197+
chunks=(10,),
198+
shards=None,
199+
dtype="float64",
200+
compressors=None,
201+
fill_value=0.0,
202+
)
203+
# Write fill values to the second chunk; assert no key created for it.
204+
arr[10:20] = 0.0
205+
assert "c/1" not in store._store_dict
206+
207+
208+
def test_S2_empty_shard_deleted_after_partial_writes_to_fill() -> None:
209+
"""S2: a sharded array where all inner chunks become fill should drop the shard."""
210+
store = MemoryStore()
211+
arr = zarr.create_array(
212+
store=store,
213+
shape=(16,),
214+
chunks=(4,),
215+
shards=(8,),
216+
dtype="float64",
217+
compressors=None,
218+
fill_value=0.0,
219+
)
220+
# Fill the first shard with non-fill data, then overwrite back to fill.
221+
arr[0:8] = np.arange(8, dtype="float64") + 1
222+
assert "c/0" in store._store_dict
223+
arr[0:8] = 0.0
224+
assert "c/0" not in store._store_dict, "shard should be deleted when fully empty"
225+
226+
227+
# ---------------------------------------------------------------------------
228+
# S3: byte-range fast path requires write_empty_chunks=True
229+
# ---------------------------------------------------------------------------
230+
231+
232+
def _is_sync_pipeline_default() -> bool:
233+
"""Check whether SyncCodecPipeline is the active pipeline."""
234+
store = MemoryStore()
235+
arr = zarr.create_array(store=store, shape=(8,), chunks=(8,), dtype="uint8", fill_value=0)
236+
return isinstance(arr._async_array.codec_pipeline, SyncCodecPipeline)
237+
238+
239+
def test_S3_byte_range_path_skipped_when_write_empty_chunks_false() -> None:
240+
"""S3: under default config, partial shard writes do not call set_range_sync."""
241+
if not _is_sync_pipeline_default():
242+
pytest.skip("byte-range fast path is specific to SyncCodecPipeline")
243+
244+
store = MemoryStore()
245+
arr = zarr.create_array(
246+
store=store,
247+
shape=(100,),
248+
chunks=(10,),
249+
shards=(100,),
250+
dtype="float64",
251+
compressors=None,
252+
fill_value=0.0,
253+
# Default config: write_empty_chunks=False
254+
)
255+
arr[:] = np.arange(100, dtype="float64")
256+
with patch.object(type(store), "set_range_sync", wraps=store.set_range_sync) as mock:
257+
arr[5] = 999.0
258+
assert mock.call_count == 0, (
259+
"byte-range fast path was taken with write_empty_chunks=False; "
260+
"this would produce a dense shard layout incompatible with empty-chunk skipping"
261+
)
262+
263+
264+
def test_S3_byte_range_path_used_when_write_empty_chunks_true() -> None:
265+
"""S3: with write_empty_chunks=True, partial shard writes use set_range_sync."""
266+
if not _is_sync_pipeline_default():
267+
pytest.skip("byte-range fast path is specific to SyncCodecPipeline")
268+
269+
store = MemoryStore()
270+
arr = zarr.create_array(
271+
store=store,
272+
shape=(100,),
273+
chunks=(10,),
274+
shards=(100,),
275+
dtype="float64",
276+
compressors=None,
277+
fill_value=0.0,
278+
config={"write_empty_chunks": True},
279+
)
280+
arr[:] = np.arange(100, dtype="float64")
281+
with patch.object(type(store), "set_range_sync", wraps=store.set_range_sync) as mock:
282+
arr[5] = 999.0
283+
assert mock.call_count >= 1, "byte-range fast path was not taken with write_empty_chunks=True"
284+
285+
286+
# ---------------------------------------------------------------------------
287+
# B1: code that mutates buffers from store IO must copy first
288+
# ---------------------------------------------------------------------------
289+
290+
291+
def test_B1_partial_shard_write_handles_readonly_store_buffers(tmp_path: Path) -> None:
292+
"""B1: LocalStore returns read-only buffers; mutating-paths must copy."""
293+
store = LocalStore(tmp_path / "data.zarr")
294+
arr = zarr.create_array(
295+
store=store,
296+
shape=(16,),
297+
chunks=(4,),
298+
shards=(8,),
299+
dtype="float64",
300+
compressors=None,
301+
fill_value=0.0,
302+
config={"write_empty_chunks": True},
303+
)
304+
arr[:] = np.arange(16, dtype="float64")
305+
# This triggers the byte-range path which decodes the shard index from
306+
# a (potentially read-only) store buffer and then mutates it. If the
307+
# decode result isn't copied, the next line raises
308+
# `ValueError: assignment destination is read-only`.
309+
arr[2] = 42.0
310+
assert arr[2] == 42.0
311+
312+
313+
# ---------------------------------------------------------------------------
314+
# Sanity: SupportsSetRange is correctly implemented
315+
# ---------------------------------------------------------------------------
316+
317+
318+
def test_supports_set_range_is_runtime_checkable() -> None:
319+
"""Stores should report SupportsSetRange membership via isinstance."""
320+
assert isinstance(MemoryStore(), SupportsSetRange)

0 commit comments

Comments
 (0)