Conversation
…, not truthiness `_ShardReader` defines `__len__`, so `shard_reader or create_empty(...)` replaced a loaded zero-chunk shard with an empty reader; the outcome was equivalent but the test meant "was a shard loaded". Likewise `get_chunk_slice` returns `tuple[int, int] | None`, and `_load_full_shard_maybe` relied on `Buffer.__len__` to treat a missing and a zero-length value alike; that conflation is now spelled out. No behaviour changes. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…very path A zero-length shard was read as missing by the async full-shard read and partial write, and failed the index checksum on every other path. The index decoder now checks the index length first, so every read and partial-write path, under either pipeline, raises the same ValueError. zarrs and tensorstore also reject such a shard: https://github.com/zarrs/zarrs/blob/b8ad3ab45082b7d55ac0dd653594953a9d9e6e7d/zarrs/src/array/codec/array_to_bytes/sharding/sharding_codec.rs#L1273 https://github.com/google/tensorstore/blob/f47412a006d7b3da888f3dbb7fc001d1ac6553e1/tensorstore/kvstore/zarr3_sharding_indexed/shard_format.cc#L113 Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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 🤖
A stored shard too short to hold its shard index, including a zero-length value, now raises a
ValueErrorsaying the shard is truncated or corrupt, on every read and partial-write path under either codec pipeline. Previously the asynchronous full-shard read and partial write treated a zero-length shard as missing (reading the fill value, or overwriting it), while every other path failed with a checksum mismatch or a decode error. zarrs and tensorstore also reject such a shard.Changes
refactor(sharding): tests shard readers and chunk byte slices withis Noneinstead of truthiness._ShardReaderdefines__len__, soshard_reader or _ShardReader.create_empty(...)replaced a loaded zero-chunk shard with an empty reader. The outcome was the same, but the check meant "was a shard loaded". This commit changes no behaviour.fix(sharding): adds_check_shard_index_size, which runs before the shard index is decoded. Every read and partial-write path decodes the index, so this one check covers all of them._load_full_shard_maybeno longer treats a zero-length value as missing.zarr-python never writes a shard shorter than its index: a shard with no chunks is deleted instead. A short value comes only from a truncated or interrupted write, an empty object created by some other tool, or another writer.
zarrs and tensorstore both reject a shard shorter than its index:
sharding_codec.rs#L1273shard_format.cc#L113Tests
test_sharding_truncated_shard_raiseschecks the error for both pipelines, memory and local stores, both index locations, zero-length and 3-byte shards, and full read, partial read and partial write: 48 cases. All 48 fail without the fix. With it,tests/test_codecs,tests/test_array.pyandtests/test_indexing.pypass (3857 passed, 770 skipped, 4 xfailed).Bookkeeping
The changelog fragment is named after this PR's number. If the change goes upstream, rename the fragment to the upstream PR's number.
🤖 Generated with Claude Code