fix: byte-order handling for structured dtypes in the bytes codec - #220
Merged
Conversation
The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5
Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5
The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5
Assisted-by: ClaudeCode:claude-fable-5
d-v-b
force-pushed
the
claude/zarr-python-4141-543977
branch
from
July 13, 2026 13:49
bfea033 to
e110bf6
Compare
d-v-b
added a commit
that referenced
this pull request
Jul 17, 2026
* chore(deps): bump the actions group across 1 directory with 8 updates (#176) Bumps the actions group with 8 updates in the / directory: | Package | From | To | | --- | --- | --- | | [prefix-dev/setup-pixi](https://github.com/prefix-dev/setup-pixi) | `0.9.5` | `0.9.6` | | [codecov/codecov-action](https://github.com/codecov/codecov-action) | `6.0.0` | `6.0.1` | | [github/issue-metrics](https://github.com/github/issue-metrics) | `4.2.2` | `4.2.7` | | [j178/prek-action](https://github.com/j178/prek-action) | `2.0.3` | `2.0.4` | | [actions/upload-artifact](https://github.com/actions/upload-artifact) | `7.0.0` | `7.0.1` | | [actions/download-artifact](https://github.com/actions/download-artifact) | `7.0.0` | `8.0.1` | | [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) | `1.13.0` | `1.14.0` | | [zizmorcore/zizmor-action](https://github.com/zizmorcore/zizmor-action) | `0.5.3` | `0.5.6` | Updates `prefix-dev/setup-pixi` from 0.9.5 to 0.9.6 - [Release notes](https://github.com/prefix-dev/setup-pixi/releases) - [Commits](prefix-dev/setup-pixi@1b2de7f...5185adf) Updates `codecov/codecov-action` from 6.0.0 to 6.0.1 - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@57e3a13...e79a696) Updates `github/issue-metrics` from 4.2.2 to 4.2.7 - [Release notes](https://github.com/github/issue-metrics/releases) - [Commits](github-community-projects/issue-metrics@c9e9838...1e38d5e) Updates `j178/prek-action` from 2.0.3 to 2.0.4 - [Release notes](https://github.com/j178/prek-action/releases) - [Commits](j178/prek-action@6ad8027...bdca6f1) Updates `actions/upload-artifact` from 7.0.0 to 7.0.1 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v7...043fb46) Updates `actions/download-artifact` from 7.0.0 to 8.0.1 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v7...3e5f45b) Updates `pypa/gh-action-pypi-publish` from 1.13.0 to 1.14.0 - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.13.0...cef2210) Updates `zizmorcore/zizmor-action` from 0.5.3 to 0.5.6 - [Release notes](https://github.com/zizmorcore/zizmor-action/releases) - [Commits](zizmorcore/zizmor-action@b1d7e1f...5f14fd0) --- updated-dependencies: - dependency-name: prefix-dev/setup-pixi dependency-version: 0.9.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: codecov/codecov-action dependency-version: 6.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: github/issue-metrics dependency-version: 4.2.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: j178/prek-action dependency-version: 2.0.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: actions/download-artifact dependency-version: 8.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: pypa/gh-action-pypi-publish dependency-version: 1.14.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: zizmorcore/zizmor-action dependency-version: 0.5.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: byte-order handling for structured dtypes in the bytes codec (#220) * fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 * docs: 3.3.0 release notes Add missing changelog fragments for zarr-developers#3955 (datetime64/timedelta64 V3 metadata types), zarr-developers#3966 (writes to 0-dimensional sharded arrays), and the public alias renames that accompanied the zarr-developers#3963/zarr-developers#3968 enum deprecations, then build the 3.3.0 release notes with towncrier, consuming all fragments accumulated since v3.2.1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
d-v-b
added a commit
that referenced
this pull request
Jul 20, 2026
* fix: byte-order handling for structured dtypes in the bytes codec (#220) * fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 * fix(store): copy buffers on write in MemoryStore Encoding an uncompressed chunk hands the store a zero-copy view of the caller's array, and MemoryStore keeps whatever it is given alive in a dict rather than serializing it. Mutating the source array after a write therefore rewrote chunks already committed to the store, silently. Stores that serialize on write (LocalStore, ZipStore, remote stores) are unaffected, so they keep the full benefit of zarr-developers#3885. Only MemoryStore pays the copy, and only where it was aliasing to begin with: an uncompressed 34 MB write goes from ~23 ms to ~39 ms, while compressed writes are unchanged. Copying at the store boundary rather than narrowing the fast path in _merge_chunk_array also fixes the single-chunk case, which aliased in v3.2.1 too. Assisted-by: ClaudeCode:claude-opus-4.8 * docs: add changelog entry for MemoryStore buffer copy Assisted-by: ClaudeCode:claude-opus-4.8 * docs: correct changelog
d-v-b
added a commit
that referenced
this pull request
Jul 21, 2026
…arr-developers#4165) * fix: byte-order handling for structured dtypes in the bytes codec (#220) * fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 * fix(store): FsspecStore.close() no longer closes the filesystem FsspecStore.close() closed the underlying filesystem's session, on the premise that a store built by from_url "owns" the filesystem it created. That premise does not hold: fsspec caches and shares filesystem instances across callers (its instance cache keys on storage options, not path), and users can hand one filesystem to many stores directly. Closing one store therefore killed the session that sibling stores were still using, and left the dead filesystem in fsspec's cache for later callers. Determining whether a filesystem is actually shared requires reaching into fsspec's private instance cache (_cache, _fs_token, cachable) and walking wrapper chains for caching/proxy filesystems — an implementation detail that leaks upward and that we would have to keep in sync with fsspec forever, getting it subtly wrong in between. The wrapper case alone (simplecache::/dir://) already slipped through a cache-membership check. The filesystem's lifecycle is simply not the store's to manage. This removes the ownership model added in the unreleased zarr-developersgh-4003: no _owns_fs, no _close_fs, no ownership transfer in with_read_only, and close() just marks the store not-open. The only thing given up is suppressing an "Unclosed client session" ResourceWarning, which was true anyway — the session belongs to a cached filesystem that outlives the store. Since zarr-developersgh-4003 never shipped (latest release is v3.2.1), its changelog fragment is removed rather than superseded. Assisted-by: ClaudeCode:claude-opus-4.8 * test: skip with_read_only fs test when AsyncFileSystemWrapper is absent test_with_read_only_shares_filesystem replaced an ownership test that carried a guard for fsspec < 2024.12.0, and the guard was dropped in the rewrite. The test still opens a file:// URL, which needs AsyncFileSystemWrapper, so it failed the min_deps job. Assisted-by: ClaudeCode:claude-opus-4.8 * docs: correct changelog claim about zarr-developersgh-4003 release status The fragment said zarr-developersgh-4003 was unreleased with no net change for released versions. Its text is already in the staged 3.3.0 release notes, so the revert is a real behavior change for anyone relying on close() releasing the session. Assisted-by: ClaudeCode:claude-opus-4.8 * docs: remove changelog entry for unreleased versions
d-v-b
added a commit
that referenced
this pull request
Jul 29, 2026
…zarr-developers#4200) * fix: byte-order handling for structured dtypes in the bytes codec (#220) * fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 * fix: make benchmark page-cache clearing opt-in, never prompt for sudo clear_cache() in tests/benchmarks/test_e2e.py ran sudo unconditionally in every benchmark's setup, so a plain `pytest` run blocked on a password prompt (zarr-developers#4199). It is now a no-op unless ZARR_BENCHMARK_CLEAR_CACHE is set, uses `sudo -n` so it can never block interactively, and the broken Darwin invocation ("&&" passed as an argument to sync) is fixed. The benchmark CI jobs set the variable to keep clearing caches. Closes zarr-developers#4199 Assisted-by: ClaudeCode:claude-fable-5
d-v-b
added a commit
that referenced
this pull request
Jul 30, 2026
* fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5
d-v-b
added a commit
that referenced
this pull request
Jul 31, 2026
…ms, ndsel wire format) (zarr-developers#4196) * fix: byte-order handling for structured dtypes in the bytes codec (#220) * fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 * feat: add the zarr-indexing package (TensorStore-style index transforms) Standalone workspace package extracted from the lazy-indexing branch (zarr-developers#3906): composable, lazy coordinate transforms (IndexTransform / IndexDomain / output maps), dependency-aware chunk resolution against a DimensionGridLike protocol, and an ndsel-conformant JSON wire format validated against the vendored conformance corpus. zarr itself does not depend on zarr-indexing yet — the runtime wiring lands separately once 0.1.0 is published. The package is numpy-only; its tests exercise chunk resolution against zarr's concrete ChunkGrid, so they run from the workspace root (uv sync --all-packages). Assisted-by: ClaudeCode:claude-fable-5 * style: conventional submodule import in the chunk-resolution tests Assisted-by: ClaudeCode:claude-fable-5 * perf(zarr-indexing): joint chunk enumeration for correlated vindex maps Candidate-chunk enumeration took the cartesian product of each correlated ArrayMap's per-dimension distinct chunk ids and relied on intersect() to filter untouched combinations. For a diagonal selection of P scattered points that is P**2 intersect calls — quadratic in the number of selected points, the same workload shape as zarr-developers#4174 (400 points: ~2.6s; 10k points: ~30min). Group correlated maps jointly instead: broadcast their per-point chunk ids, take the distinct rows (np.unique(axis=0), O(P log P)), and enumerate exactly the touched combinations. Candidate slots now carry chunk-coordinate tuples covering one or more output dimensions; orthogonal/constant/slice dimensions keep their existing per-dimension candidates. 400-point diagonal resolution drops from 2628ms to 14ms and scales linearly. Assisted-by: ClaudeCode:claude-fable-5 * docs(zarr-indexing): standalone documentation site; add package justfile Mirror the treatment zarr-metadata received in zarr-developers#4208/zarr-developers#4210 onto zarr-indexing: a self-contained mkdocs site under the package (own mkdocs.yml, landing page, ndsel wire-format guide, mkdocstrings page per module, and .readthedocs.yaml for a dedicated RTD project), so the package presents as a separate project with docs versioned by its own zarr_indexing-v* release tags rather than zarr-python's. The zarr-python site's API Reference nav links out to it, and each RTD project now skips PR builds that do not touch its half of the repo. The package gains a pinned docs dependency group, a docs build job in its CI workflow, and a justfile with package-scoped dev recipes. Two recipes deviate from the zarr-metadata original by design: - `test` runs against the workspace-root environment (`uv run --project ../.. --all-packages --group test`), because the chunk-resolution tests exercise this package against zarr's chunk grids and `zarr` is deliberately not a dependency of this package. - `typecheck` uses plain `pyright`, unpinned and on the default interpreter, mirroring this package's own CI invocation. The zarr-metadata pin exists for a PEP 661 sentinel regression that zarr-indexing's sources do not hit. composition.py gains the module docstring the other modules already have, since mkdocstrings renders it as the page introduction. Assisted-by: ClaudeCode:claude-fable-5 * chore: drop the already-released 4141 changelog fragment The bytes-codec byte-order fix this fragment describes shipped upstream and its entry is already in docs/release-notes.md; the fragment survived on this branch only as a rebase remnant, and would emit a duplicate entry in the next release. Assisted-by: ClaudeCode:claude-fable-5 * docs(zarr-indexing): canonicalize ndsel references to zarr-developers/ndsel Also aligns the zarr-indexing workflow's setup-uv pin (v8.3.2) with the rest of the repo. The vendored-corpus sha is present upstream; the historical d-v-b/ndsel#1 PR reference stays as provenance. Assisted-by: ClaudeCode:claude-fable-5 * chore: drop the root uv-workspace wiring for zarr-indexing Per review: the root pyproject.toml should not change in this PR. The package now operates fully standalone (like zarr-metadata); the test invocations layer the package into the repo-root environment as an editable overlay instead (python -m pytest, since a base-env console script would not see the overlay). Assisted-by: ClaudeCode:claude-fable-5
d-v-b
added a commit
that referenced
this pull request
Aug 11, 2026
* fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5
d-v-b
added a commit
that referenced
this pull request
Aug 12, 2026
* implement store server and node server * update tests and add v2 -> v3 example * add __all__ and clean up tests * add docs and changelog * add proper server * rework examples (simplify) and make server a context manager * minor tweaks to server * fix: byte-order handling for structured dtypes in the bytes codec (#220) * fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 * fix: adapt server branch to current starlette/uvicorn starlette 1.3 deprecated using httpx with its TestClient, which the warnings-as-errors filter turns into a collection error; add httpx2 to the test dependency group. uvicorn 0.51 removed Server.install_signal_handlers and now skips signal-handler setup off the main thread natively, so drop the monkey-patch workaround. Assisted-by: ClaudeCode:claude-fable-5 * feat(zarr-server): scaffold packages/zarr-server Assisted-by: ClaudeCode:claude-fable-5 * feat(zarr-server): move server code, tests, and example into the package The moved code now uses only public zarr API: zarr.buffer.cpu replaces zarr.core.buffer.cpu, spec-defined key names are inlined, chunk-key encodings are duck-typed on their spec names, and the shard grid shape is computed locally from public Array attributes. Also mirrors the root repo's [tool.numpydoc_validation] override in the package's pyproject.toml, since numpydoc-validation resolves config from the nearest pyproject.toml and would otherwise apply its stricter default checks to the moved docstrings. Assisted-by: ClaudeCode:claude-fable-5 * docs(zarr-server): package README; retarget root changelog fragment at the core fix Assisted-by: ClaudeCode:claude-fable-5 * refactor!: remove the HTTP server from zarr core The server now lives in packages/zarr-server (published as zarr-server). The feature never shipped in a zarr release, so there is no deprecation shim. The decode_chunk_key strictness fix stays in zarr core. Assisted-by: ClaudeCode:claude-fable-5 * ci(zarr-server): add package test and release workflows Assisted-by: ClaudeCode:claude-fable-5 * chore(zarr-server): refresh package lockfile after root server-extra removal The package lock embeds the workspace-root zarr project's metadata; Task 4 removed the root server extra and httpx2 test dep after this lock was first generated. Assisted-by: ClaudeCode:claude-fable-5 * fix: reject path-traversal segments in zarr-server request handling store_app (and thus serve_store) passed the request path straight to the store without validation, since the is_valid_node_key gate only ran for node_app. Starlette percent-decodes path params, so a request like GET /..%2fsecret.txt arrived as literal ".." and LocalStore resolved it outside the store root, allowing arbitrary file read via GET and arbitrary file write via PUT. Closes this path-traversal vulnerability by rejecting any "." or ".." path segment in _handle_request before the store is touched, for both store_app and node_app. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-server): close absolute-key path-traversal bypass in request guard The path-traversal guard in _handle_request only rejected "." and ".." segments. A percent-encoded leading slash (e.g. "/%2fetc%2fhostname") decodes to an absolute path param ("/etc/hostname"), whose split() produces an empty leading segment with no "." or ".." segment, so the guard let it through. LocalStore resolves an absolute key by discarding its configured root, allowing arbitrary filesystem read/write outside the store. Reject empty segments too, closing the bypass. Assisted-by: ClaudeCode:claude-fable-5 * ci(zarr-server): bump actions/attest to v4.2.0 to match sibling workflow Keeps the zarr-server release workflow's pinned action SHAs in sync with the dependabot bump that landed in zarr-metadata-release.yml via the merge. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-server): fold backslash separators and reject drive-qualified keys The traversal guard in _handle_request split only on "/", so backslash- separated segments like "..\\..\\win.ini" and drive-qualified or UNC-rooted keys like "C:/Windows/win.ini" or "\\host\share\x" passed through untouched. On Windows, LocalStore joins keys onto its root via pathlib, which discards the root entirely for a drive-qualified or rooted key -- turning percent-encoded backslash paths into arbitrary file read/write. Fold backslashes to "/" before the segment check (mirroring zarr's own normalize_path) and add an ntpath.splitdrive check to catch drive letters and UNC prefixes that don't produce empty/"."/".." segments. Added TDD coverage that fails against the old guard (via a store double that raises if get/set is ever called) and passes against the fix. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-server): fix vacuous two-level traversal test The `/..%2f..%2fsecret.txt` case in test_encoded_traversal_variants_ return_404 climbed two levels from tmp_path/store_root to tmp_path/.., but the secret was written at tmp_path/secret.txt (one level up) -- so the case passed for the wrong reason (no such file, not "guard blocked it") even with the traversal guard deleted entirely. Nest the store root exactly `climb_depth` directories below tmp_path per case, so every case's ".." segments resolve to tmp_path/secret.txt. Verified by mutation: copied _serve.py to a scratch dir (never the repo), deleted the guard body, and ran TestPathTraversalProtection against it via PYTHONPATH. Before this fix, 12/13 traversal tests failed against the mutant (1 false pass -- this vacuous case). After this fix, 13/13 fail against the mutant, and all still pass against the real guard. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-server): bound BackgroundServer shutdown instead of blocking forever shutdown() joined the server thread with no timeout, and uvicorn's graceful wait is itself unbounded: force_exit is only set by a signal handler uvicorn deliberately skips off the main thread. A client mid request could wedge __exit__ unrecoverably. Bound uvicorn's graceful wait with timeout_graceful_shutdown and fall back to force_exit if the thread outlives it, both driven by a new shutdown_timeout parameter on serve_store/serve_node. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-server)!: only accept HTTP methods the handler implements _handle_request special-cased PUT and let every other verb fall through to the read path, so a server built with DELETE/POST/PATCH answered them with the key's contents and changed nothing. HTTPMethod advertised all of them. Narrow HTTPMethod to GET/PUT/HEAD and reject anything else when the app is built. Doing this before the first release avoids narrowing a published type later. Assisted-by: ClaudeCode:claude-fable-5 * docs(zarr-server): document client deps; lint the package's changelog in CI The README's round-trip example reads back through FsspecStore, which needs an HTTP-capable fsspec that zarr-server does not depend on, so a clean install failed on the PyPI landing page's headline snippet. Same for examples/serve.py under a plain interpreter, which needs httpx. check_changelogs.yml also never visited packages/zarr-server/changes, so a malformed fragment would have passed PR CI and failed at release time. Assisted-by: ClaudeCode:claude-fable-5 * test(zarr-server): add docs dependency group and cover the README round-trip Reading a served array back with zarr.open_array(url) routes through FsspecStore -- zarr's only URL-string backend -- so the README's headline example needs an HTTP-capable fsspec that the package itself has no reason to depend on. Carry that in a docs group, mirroring the root project's group for running examples, and use it to test the round-trip in CI so the example cannot rot. Assisted-by: ClaudeCode:claude-fable-5 * refactor(zarr-http-server)!: rename zarr-server to zarr-http-server The package is HTTP-specific end to end -- Starlette/ASGI, byte-range headers, CORS, HTTP verbs -- so the unqualified name overclaimed its scope and squatted the generic name that a future transport (an S3-compatible or WebDAV frontend) would want. Renames the distribution, the zarr_http_server module, the package directory, the zarr_http_server-v* release tags, both workflows and their artifact and PyPI environment names. Nothing is published yet, so this costs nothing now and would be permanent after the first upload. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-http-server): reject an empty methods set instead of failing open Starlette's Route treats a falsy `methods` as "match every method", and an empty set passes the unsupported-verb check trivially, so store_app(store, methods=set()) served GET, DELETE, POST, PATCH and TRACE alike and accepted PUT writes -- the opposite of what the caller asked for, and the same fail-open the method narrowing set out to close. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-http-server): report the bound port, unblock the loop, 404 unopenable children Three defects the adversarial re-review reproduced: BackgroundServer reported the requested port, so port=0 produced http://host:0 while the socket was bound elsewhere -- clients following the documented url reached an unrelated service. Group key validation opened children through zarr's synchronous API directly on the event loop, serializing every concurrent request behind it (20 concurrent requests: 10.5s, now 1.1s) and outlasting the shutdown timeout. Move it to a worker thread. Child lookup caught only KeyError, so a corrupt metadata document or a codec from an uninstalled plugin surfaced as 500 and made a whole subtree unservable during ordinary operation. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-http-server): close the S5-S13 follow-ups from the adversarial review Hostile keys the store cannot express (embedded NUL, over-long names) now answer 404 instead of surfacing ValueError/OSError as a 500. PUT bodies are capped at DEFAULT_MAX_BODY_SIZE and answer 413 past it; Store.set takes a whole Buffer, so a body cannot be streamed and one request would otherwise size the server's memory. 206 responses carry Content-Range, and a range beyond the end or an inverted one answers 416 rather than an empty 206. A 0-d v2 array's sole chunk, stored under "0", is now servable: the key decodes to a 1-tuple no 0-d grid could match, so it needs the array's dimensionality to disambiguate. Array and group metadata key sets are split, so a v2 group no longer claims .zarray as its own; bind failures report the likely cause instead of a bare timeout; and serve_* gain a background: bool overload. The two out-of-bounds chunk tests planted no data, so they passed for the wrong reason -- mutating the bounds check to return True left the suite green. They now plant data at the out-of-grid key, and that mutation fails 3 tests. Adds the missing coverage for this branch's one core change, and widens the workflow's path filter to src/zarr, since the package resolves zarr from the repo root and a core change can break it. Assisted-by: ClaudeCode:claude-fable-5 * fix(zarr-http-server): stop reporting I/O failures as misses; enforce the body cap while reading The round-3 review found the previous round's error handling was too broad and its body cap too narrow. except (ValueError, OSError) around store.get and store.set caught the whole errno family, so EACCES, EROFS and ENOSPC all answered 404. Under the v3 spec an absent chunk is an uninitialized one and a reader is right to substitute the array's fill value, so 404 asserts something about the store's contents: an unreadable chunk answered that way has a correct client silently materialize fill values over data that exists, and a failed write looks like a pointless one rather than a failure. Only ENAMETOOLONG and EINVAL now mean "this key names nothing"; everything else surfaces. NUL bytes are rejected in the guard instead, so the store's own errors always mean real I/O trouble. max_body_size only consulted Content-Length, which a chunked request does not send, so request.body() buffered the whole thing before the check: 256 MiB passed a 1 KiB cap at 572 MiB peak RSS. The body is now read incrementally and abandoned at the cap -- the same attack peaks at 59 MiB. Also: a range too wide to allocate answers 416 rather than raising MemoryError; the force_exit fallback join is bounded, so shutdown cannot outlast its timeout; DEFAULT_MAX_BODY_SIZE is importable, since it is the documented default of three public functions; and _make_starlette_app loses a parameter it never read. Adds the coverage the review found missing: shard-grid bounds (the mutant served an out-of-grid shard key against a green suite), wrong-arity chunk keys, the chunked body path, I/O failures as 5xx, and a parser-level assertion for inverted ranges, which the status code alone could not distinguish on a MemoryStore. The drive-letter check stays unconditional. It over-rejects a first-segment node name like a:b, which is legal on POSIX -- but ntpath treats any single character before a colon as a drive, so there is no safe subset, and the guard is a string gate in front of an arbitrary Store whose path semantics this package cannot know. Assisted-by: ClaudeCode:claude-fable-5 * fix(ci): silence BLE001 on two intentional broad excepts ruff 0.16 selects BLE001 under the root config's `B` prefix, so the package's `uvx ruff check .` job failed on two deliberate blind excepts. Both are intentional and already documented, so they get targeted noqa comments, matching how zarr-developers#4213 handled the same rule in the core tree. Assisted-by: ClaudeCode:claude-opus-5 * chore: drop stale changelog fragment for the chunk key fix The DefaultChunkKeyEncoding.decode_chunk_key fix this fragment described was split out into zarr-developers#4219 and has already shipped -- its text is in docs/release-notes.md verbatim. Leaving the fragment here would emit the same paragraph a second time under a zarr-developers#3732 link, for a change this branch no longer contains. Assisted-by: ClaudeCode:claude-opus-5 * ci: check zarr-http-server changelogs and align action pins check_changelogs.yml validated the root, zarr-metadata and zarr-indexing changes/ directories but not zarr-http-server's, so the new package's fragments were the only ones whose filenames went unchecked. The two new workflows were also written before zarr-developers#4241 bumped the actions group, so they pinned older checkout/setup-uv/attest/pypi-publish SHAs than their siblings. Bump them to the versions main already uses; the release workflow is now identical to zarr-metadata's modulo the package name. Assisted-by: ClaudeCode:claude-opus-5 * docs(http-server): add justfile and Read the Docs scaffold Brings the package in line with zarr-metadata and zarr-indexing, which each own a justfile and a separate Read the Docs site. The docs content is a scaffold -- an overview page and an API reference over the public namespace -- meant to be filled in later; the point is to get the site wired up and building under --strict now. The `docs` dependency group previously held the runtime deps for the README examples, but Read the Docs and `just docs-check` both expect it to carry the mkdocs toolchain, as it does in the sibling packages. Those example deps move to a new `examples` group and the test job follows. CI gains a `docs` job matching the siblings', so a scaffold that stops building fails the gate. The repo-root .readthedocs.yaml skips PR builds confined to this package now that it has its own site, and the root docs nav links out to it. Creating the Read the Docs project itself is a manual step: point its configuration-file path at packages/zarr-http-server/.readthedocs.yaml. Assisted-by: ClaudeCode:claude-opus-5 * fix(http-server): correct silent-corruption and HTTP-conformance defects Two of these answered a request successfully while leaving the client with data that does not exist. Chunk keys were validated by decoding coordinates and bounds-checking them, never by re-encoding. `int` is lenient in ways a store key is not -- leading zeros, a leading `+`/`-`, surrounding whitespace, underscore separators, non-ASCII decimal digits -- so `c/00/00` decoded to (0, 0) and validated, then went to the store verbatim. A PUT answered 204 and stored the body under a key no reader looks up: success reported, data invisible. Validation now requires the key to equal `metadata.encode_chunk_key(coords)`, which makes the accepted set exactly the set zarr can read. Decoding delegates to the encoding's own decoder rather than reimplementing the default/v2 grammars, so a new or third-party chunk key encoding works without changes here. Resolving a group child caught bare `Exception` and returned False, which turned an unreadable child -- EACCES, EIO, a corrupt metadata document, a missing codec plugin -- into 404. Under the v3 spec an absent chunk is an uninitialized one, so a correct reader answers that 404 by substituting the fill value over data that exists. Only KeyError is caught now; a key that could not be judged surfaces as 5xx rather than being reported absent. The rest are conformance fixes on the same request path: - A Range header the server cannot use is now ignored with a 200 rather than refused with a 416, per RFC 9110 §14.2. This covers an unrecognized unit and a multi-range request, both legal to send. - A suffix range resolves against the object's size, so a 206 always carries the Content-Range that RFC 9110 §15.3.7 requires. Sharding reads a shard index this way, so the header was missing on a hot path. - A last-byte-pos wider than the store can materialize is clamped to the end of the object per §14.1.2 instead of raising out of the store as a 500. - A byte position is parsed as 1*DIGIT rather than by `int`, which accepted `+0`, ` 0` and `0_0`. - PUT to a read-only store answers 403 instead of letting the store's ValueError surface as a 500. Assisted-by: ClaudeCode:claude-opus-5 * test(http-server): property tests over a real endpoint Adds hypothesis properties that drive a real uvicorn server over a socket, so the assertions cover what only exists on the wire: header parsing, method dispatch, status codes. Each property checks the response *and* the backing store. That pairing is what the previous suite could not do: a PUT to a non-canonical chunk key answered 204 and wrote a key no reader consults, which a response-only assertion cannot see. Refused requests assert the store is byte-for-byte unchanged; accepted writes assert the bytes landed under the key the client named and that a zarr client reads back the values. Keys are generated in two families -- in-band (the node's metadata and the canonical spelling of each chunk key in its grid) and out-of-band (non-canonical spellings, out-of-grid coordinates, traversal probes, a sibling node's keys). Two details worth keeping: - Traversal probes are percent-encoded. An HTTP client resolves dot-segments before sending, so httpx turns "../secret" into "/secret" and a literal probe asserts nothing; encoded, it reaches the server and Starlette decodes it back into a real ".." segment. - The matrix includes LocalStore, not just MemoryStore. MemoryStore slices a `bytes` and accepts any range bound, so it cannot distinguish a clamped over-wide range from a refused one -- mutation-testing the suite showed that property passing against deliberately broken code until a filesystem-backed server was added. Verified by reverting each fix in turn and confirming the corresponding property fails. Assisted-by: ClaudeCode:claude-opus-5 * fix(http-server): stop reading EINVAL from a store as a missing key `_names_nothing` reclassified two errnos as absence so a client could not turn a freely chosen key into a 5xx. `ENAMETOOLONG` earns that: it is the store answering about the name -- nothing can be stored under a name it cannot express -- and `encode_chunk_key` never produces a segment near a filesystem's length limit, so it is unreachable for real data. `EINVAL` does not. It is POSIX's catch-all, reachable on a perfectly ordinary short key through a bad seek or an unsupported filesystem feature, and under the v3 spec an absent chunk is an uninitialized one -- so answering 404 has a correct reader write fill values over a chunk that exists but could not be read. That is the same defect already fixed in the group-child lookup and in chunk key validation: reporting "something went wrong" as "it is not there". Nothing exercised EINVAL in practice. A NUL in a key raises ValueError and is rejected before the store anyway, and an over-long key raises ENAMETOOLONG, so this narrows the guard to the case that was doing the work. The traversal, absolute-key and drive-letter guards deliberately stay where they are rather than deferring to the store. LocalStore.get/set are `self.root / key` with no validation -- zarr's normalize_path applies at the StorePath layer, not to raw store keys -- so `../sibling.txt` and an absolute key both write outside the store root. This server is the component that feeds a Store unvalidated strings from the network, so it is the component that has to reject them. Assisted-by: ClaudeCode:claude-opus-5 * feat(http-server): stop sealing over CORSMiddleware and uvicorn.Config Wrapping an API means taking responsibility for its parameters, not hiding the ones we did not think to name. Two wrappers were doing the latter. `CorsOptions` carried 2 of `CORSMiddleware`'s 8 parameters, so `allow_headers`, `allow_credentials`, `allow_origin_regex`, `allow_private_network`, `expose_headers` and `max_age` were unreachable without bypassing this package. It now mirrors the full signature, with every key optional. Two of the defaults are ours rather than Starlette's, because the server knows what its caller should not have to. It emits `Content-Range` on every ranged response, which is not a CORS-safelisted response header, so `expose_headers` defaults to `["Content-Range"]` -- without it a browser client could read the bytes but not learn which bytes it got, which made the suffix-range Content-Range fix invisible to exactly the clients CORS exists for. It accepts a `Range` request header, so `allow_headers` defaults to `["Range"]`; Starlette's empty default answered a preflight naming Range with 400. Defaults apply only to absent keys, so an explicit `expose_headers: []` means "expose nothing". `_start_server` passed 4 of `uvicorn.Config`'s 52 parameters, which put TLS, `proxy_headers`/`forwarded_allow_ips`, `root_path`, `log_level`, `limit_concurrency` and unix-socket binds out of reach entirely. A `uvicorn_options: Mapping[str, object] | None` is merged over the three options set here, so a caller key wins. uvicorn ships no TypedDict for Config -- its only TypedDicts are ASGI protocol events -- so the mapping is hand-typed and the cast is confined to the call site. Un-sealing uvicorn makes two BackgroundServer attributes reachable that could previously only be one thing. `url` now reports the scheme actually in use, so configuring TLS yields https, and `host`/`port`/`url` are None for a uds or fd bind rather than naming an address nothing is listening on. Assisted-by: ClaudeCode:claude-opus-5 * fix(http-server): lifecycle, HEAD cost, and CORS/route method agreement Clears the findings left open from the review. Lifecycle. `_start_server` raised on its startup timeout without ever signalling the server, so the thread went on to bind the port and serve forever as a daemon with no handle to stop it -- and a retry on the same port then failed with the other error. It now sets should_exit and force_exit and joins before raising. `shutdown()` returned normally when the thread survived both joins, reporting success for a server still bound and still serving; it now raises. The first join also matched uvicorn's own `timeout_graceful_shutdown` exactly, and uvicorn spends ~0.2s tearing down before that wait even begins, so the join always expired first and escalated to force_exit on the orderly path -- which makes uvicorn skip ASGI lifespan shutdown. The join now outlasts the graceful bound by a margin, and reads that bound from the config so it stays correct when a caller sets it through uvicorn_options. HEAD. A HEAD body is discarded at the wire, but HEAD fell through to the GET handler, so answering one transferred the whole value: measured at 10 MB read to report a length. It is now answered from `Store.getsize` -- a stat on a filesystem store, an info call on a remote one -- and reads zero bytes. HEAD is served whenever GET is, which is what Starlette does and what RFC 9110 asks of an origin server; the README and docstrings said otherwise and now say so. CORS. `cors_options["allow_methods"]` was passed through unchecked, so an app could advertise methods its route rejects: a browser caches that preflight and every later cross-origin call fails with 405 after a successful handshake. Advertising an unserved method is now a ValueError at construction, consistent with how unsupported `methods` are already rejected, and `"*"` expands to what is actually served rather than to every verb Starlette knows. An absent `allow_methods` is left alone -- widening it to everything served would newly advertise PUT cross-origin on a write-enabled app that never asked for it. Media type. The JSON content type was keyed off a third hardcoded "zarr.json", so a v2 array's `.zarray` was served as octet-stream. It is now derived from the same tables that decide which keys a node owns. Also documents that `store_app` does not validate keys: it proxies the raw key space and has no array semantics to check against, so a client that misspells a chunk key gets a successful write to a key no reader consults. `node_app` rejects that with 404. Assisted-by: ClaudeCode:claude-opus-5 * ci(http-server): run the justfile's recipes instead of copies of them The workflow repeated the commands the justfile already defines -- `uvx ruff check .` and the mypy invocation were byte-identical copies, and the pytest step differed only by the sync that precedes it. Two definitions of the same verb drift silently: renaming the `docs` dependency group to `examples` required the same edit in both places, and updating only one would have left `just check` and CI testing different things with nothing failing. CI now calls `just test`, `just lint` and `just typecheck` (it already called `just docs-check`), keeping the python matrix and caching, which are genuinely CI's concern. This matches zarr-metadata, whose workflow already states the arrangement; zarr-indexing remains half-converted. Delegating also meant fixing what the shared recipe would otherwise spread: `just lint` ran an unpinned `uvx ruff`, which is precisely how this job broke before -- ruff 0.16 began selecting BLE001 under the root config's `B` prefix and failed on rules the pre-commit-pinned ruff never enforced, with no code change to blame. The recipe now pins the same version .pre-commit-config.yaml does, so the local gate, the pre-commit gate and CI enforce one standard. Assisted-by: ClaudeCode:claude-opus-5 * ci: build the zarr-http-server docs on pre-push A dead cross-reference or a nav entry pointing at a removed file only fails at `mkdocs build --strict`, which until now happened first in CI. This catches it before the code leaves the machine. Scoped deliberately. `stages: [pre-push]` overrides the repo default of running on every commit: this builds the whole site, which is too slow to pay per commit and is only actionable before pushing. `files:` limits it to changes that touch the package, and `pass_filenames: false` because mkdocs builds a site rather than a list of files. It delegates to `just docs-check` so the build has one definition shared with CI, and is added to `ci.skip` alongside mypy for the same reason that one is skipped: pre-commit.ci's runners have neither `uv` nor the repo checkout needed to resolve the environment. The zarr-http-server workflow covers it there. Note this hook and CI still declare their toolchains separately -- the hook shells out to the local `just`/`uv`, CI installs them itself. That is inherent to pre-commit.ci not being able to run them, and is the same trade already accepted for mypy. Also refreshes packages/zarr-http-server/uv.lock, which references the root project's dependency groups, for the hypothesis and uv bumps that arrived with the main merge. Assisted-by: ClaudeCode:claude-opus-5 * feat(http-server): make the read-only guarantee explicit and enforced Read-only was already the default -- `store_app(store)` answers 405 to PUT, POST, DELETE and PATCH, and POST is unconfigurable because there is no handler behavior for it -- but nothing said so and little pinned it. Only PUT was covered against the default app; POST, DELETE and PATCH were covered only against a fixture built with writes enabled, so "the default app is read-only" was not actually a tested claim. Adds a test class covering both layers the guarantee rests on: `methods`, which decides what the route answers, and the store, which decides whether a write could succeed at all. Each refusal also asserts the value is unchanged, matching the property tests -- a 405 that still wrote would otherwise pass. Serving PUT from a read-only store is now a ValueError at construction. A store's `read_only` is fixed when it is built, so that combination can never succeed; it previously surfaced as a 403 to whichever client tried to write first, long after whoever misconfigured it had moved on. The handler's 403 stays as a backstop for a store whose read_only is not fixed, and the test for it builds the app through the private builder since the public entry points now reject the combination. Documents `store.with_read_only(True)` as the categorical recipe: it is the stronger of the two layers because it holds even if the HTTP layer is misconfigured. Assisted-by: ClaudeCode:claude-opus-5 * feat(http-server): name the read-only and read-write method sets `READ_ONLY_METHODS` and `READ_WRITE_METHODS` let a call site say which it is, rather than leaving that to the presence or absence of an argument. The read-only one is exactly the default, so passing it changes nothing except that the intent is written down. The value is the other direction: a writable app must name a method set, so `grep -r 'methods='` finds every place that opts into writes -- which is what makes a deployment auditable without a separate read-only entry point. Both are frozensets, so one caller cannot widen the default for every other, and both name HEAD explicitly: Starlette serves it wherever GET goes, and a constant that omitted it would misdescribe the route. `methods` now accepts any `AbstractSet`, which is what lets a frozenset constant be passed where a `set` was previously required. Assisted-by: ClaudeCode:claude-opus-5 * feat(http-server): model read-only methods in the type domain Renames the constants to READ_ONLY_HTTP_METHODS / READ_WRITE_HTTP_METHODS so they say what kind of method they hold, matching the HTTPMethod type they are drawn from. Adds `ReadOnlyHTTPMethod = Literal["GET", "HEAD"]`, which moves the distinction from a runtime convention to something a checker enforces: a `frozenset[ReadOnlyHTTPMethod]` cannot contain "PUT", so a read-only interface can be declared rather than merely configured. Verified against mypy --strict -- assigning either a set containing "PUT" or READ_WRITE_HTTP_METHODS to that annotation is an error, while READ_ONLY_HTTP_METHODS is accepted. HTTPMethod is now the union of that and a private `_WriteHTTPMethod` rather than a third hand-written list of the same strings, and both constants plus _SUPPORTED_METHODS are derived from the Literals via get_args. The runtime sets and the static types therefore cannot disagree about what this server serves: widening a Literal is the only edit needed, and a test pins the contents so that widening is deliberate. Assisted-by: ClaudeCode:claude-opus-5 * docs(http-server): add a notebook example, and run both examples in tests The only example used `with serve_node(...)`, which is the one form that cannot work in a notebook: it shuts the server down when the cell ends, so anyone copying it gets a dead server by the next cell. Nothing in the package mentioned notebooks at all. Adds examples/serve_notebook.ipynb covering the lifecycle a kernel needs -- start with background=True and keep the handle, use it across cells, then shutdown() -- plus metadata and chunk reads, a byte range, and a refused PUT. Two arguments carry it: background=True runs uvicorn in a daemon thread with its own loop so the kernel's loop is untouched, and port=0 means re-running a start cell picks a new port instead of failing with "address already in use". A README section says the same in prose. The notebook is executed by the suite through nbclient, in a real kernel, and asserts its own expectations, so a behavior change fails there rather than in someone's notebook. Verified by mutation: making writes the default breaks the notebook's `assert refused.status_code == 405` and surfaces as a CellExecutionError naming the cell. examples/serve.py is now executed too, which required fixing the same fixed-port footgun the notebook section warns about -- it bound 8000, so it failed if anything else held that port. It runs in-process rather than under `uv run`, because its inline script metadata resolves zarr-http-server from git and would test main instead of the working tree. Assisted-by: ClaudeCode:claude-opus-5 * feat(http-server): expose serve() for arbitrary ASGI apps Serving two nodes did not need two servers, but the only way to run several was to reach past this package: `serve_store`/`serve_node` each take exactly one store or node, so a composed app had no route to the background-server ergonomics -- `port=0` into `server.url`, and `shutdown()` -- only a blocking `uvicorn.run`. `_start_server` already did this for any Starlette app; it was just private. `serve(app, ...)` makes it public, with the same background/blocking overloads the shorthands have. `serve_store` and `serve_node` now delegate to it and stay, because they are the common case and are what the docs and examples use; retiring them is still available later. The split the pair muddles is now visible: what an app *serves* (`methods`, `cors_options`, `max_body_size`) is settled when the app is built, and `serve` only decides how it runs. Documents the three ways to serve several nodes -- serve their common parent group, serve the whole store, or mount separate apps and run the result -- with tests covering mounted nodes in *separate* stores, that each mount serves only its own data, and that `serve` runs the composed app in the background. Also derives the bounded-shutdown test's threshold from the timeouts that produce it. It hard-coded 3.0s, which was generous when shutdown could take at most 2x shutdown_timeout and marginal once the join margin was added -- it began failing under load rather than at the moment the constant changed. Assisted-by: ClaudeCode:claude-opus-5 * refactor(http-server)!: split blocking and background into two functions `background: bool` decided whether a call returns immediately with a handle or never returns at all -- the largest difference a call site can have, hidden in a keyword. The return type depended on it too, which is why every runner carried three @overload stanzas: nine in total, all of them working around that one flag, and `background=False` returned None, a value meaningless half the time. `serve(app)` now blocks and `serve_background(app)` returns a BackgroundServer. Neither needs an overload. Splitting forced the shorthand question, since the axes multiply: keeping serve_store/serve_node alongside two modes means six runner functions. The public surface is instead two builders and two runners -- `serve_background(store_app(store))` replaces `serve_store(store, background=True)`. That is one more call, and it puts the two halves where they belong: what an app serves is settled when it is built, and the runner only decides how it runs. _serve.py drops from 1301 to ~1100 lines with the duplicated signatures and docstrings gone. `serve_background` defaults to `port=0` where `serve` defaults to 8000. Deliberate: a background server is reached through `server.url`, and a fixed default makes starting a second one -- or re-running a notebook cell -- fail on a collision, while a blocking server usually wants a port others already know. BREAKING CHANGE: serve_store and serve_node are removed. The package is unreleased, so nothing depends on them yet. Assisted-by: ClaudeCode:claude-opus-5 * feat(http-server): default both runners to port="auto" `serve` defaulted to 8000 and `serve_background` to 0, which read as an arbitrary disagreement between two sibling functions about a shared parameter. Both now default to `"auto"`: prefer 8000, fall back to any free port if it is taken, and report the result through `server.url` and uvicorn's own startup line. What makes the fallback safe is that it applies only to the default. An explicit port still binds exactly that or fails, because a caller who names one usually has a proxy or a container port mapping expecting the server there -- silently moving would break it while looking healthy. `port=0` keeps its OS meaning of "any free port, no preference". The port is bound here and the socket handed to `Server.run(sockets=...)` rather than probing for a free port and passing uvicorn the number: probing releases the port before uvicorn claims it, which is the bind-then-close race that makes "find a free port" helpers flaky. Holding the socket means nothing can take it in between. `Config.port` is set to what was actually bound, so uvicorn's "running on ..." line does not name a port it is not serving. Two cases the mechanism has to respect: a `uds` or `fd` bind in uvicorn_options skips the TCP bind entirely, and the address family comes from `getaddrinfo` rather than a hard-coded AF_INET, which would bind the wrong family for an IPv6 host. Both are covered by tests, as is that an explicit taken port still raises. Assisted-by: ClaudeCode:claude-opus-5 * revert: drop changes to files this package does not own The PR should not reach outside packages/zarr-http-server and .github, and four files did. Two were unrelated churn: docs/api/zarr/experimental.md renamed a heading in the *core* zarr docs about zarr.experimental.cache_store, left over from when this server lived at zarr.experimental.serve, and uv.lock carried an idna bump nothing here asked for. Reverted; `uv lock --check` is clean. Two were premature rather than wrong. mkdocs.yml added a nav link to zarr-http-server.readthedocs.io and .readthedocs.yaml skipped the repo-root docs build for changes confined to this package. Both belong with a Read the Docs project that does not exist yet -- until it does, the nav link 404s and the build skip means neither site builds the package's docs. mkdocs.yml is also what GitHub reported a conflict on, which is what surfaced this. They should land in the follow-up that creates the RTD project. What remains outside the package is two root files that have nowhere else to live: the pre-push docs hook, since a pre-commit hook is necessarily repo-level, and a comment-only change to pyproject.toml noting that the release workflow's `zarr_http_server-v*` tags are among those the `git describe --match v*` filter exists to exclude. Assisted-by: ClaudeCode:claude-opus-5
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.
🤖 AI text below 🤖
Summary
Fixes zarr-developers#4141: the
bytescodec silently corrupted structured-dtype data whose field byte order differed from the stored one.Two defects, both in
BytesCodec:self.endianagainst the buffer's top-level byteorder, but numpy reports'|'for void dtypes, so struct fields were never swapped — big-endian input was written raw underendian: littlemetadata, corrupting even a pure zarr write/read roundtrip.endianfor scalar (HasEndianness) dtypes and viewed struct chunks in the fields' native byte order — the silent-corruption path for virtual references to external big-endian data (e.g. FITS binary tables via VirtualiZarr).The fix keeps stored and in-memory byte order independent, per the struct data type spec (field data types carry no byte order; the codec's
endiangoverns all multi-byte fields):newbyteorder(self.endian), converting to the stored order when they differ.Note: v3 metadata carries no per-field byte order, so a reopened array always declares native-endian fields and reads now return correctly byte-swapped native values. Per-field endianness of the dtype declaration still doesn't survive a metadata roundtrip — that would be a metadata/API feature, distinct from this corruption fix.
For reviewers
The decode-side contract change (decoded buffers now always come back in the declared dtype's byte order, converting when needed) is the part worth a second look — it slightly changes what
_decode_syncreturns for non-native scalar dtypes, though the user-visible result was already cast by the out-buffer assignment. Encode-side and the tests are straightforward.Author attestation
TODO
docs/user-guide/*.md(n/a)changes/🤖 Generated with Claude Code