fix: FusedCodecPipeline must apply outer AA/BB codecs on partial paths - #255
Closed
d-v-b wants to merge 3 commits into
Closed
fix: FusedCodecPipeline must apply outer AA/BB codecs on partial paths#255d-v-b wants to merge 3 commits into
d-v-b wants to merge 3 commits into
Conversation
d-v-b
force-pushed
the
fix/fused-partial-aa-bb
branch
from
July 29, 2026 11:46
f678b0c to
ad67205
Compare
d-v-b
force-pushed
the
fix/fused-partial-aa-bb
branch
from
July 29, 2026 13:28
ad67205 to
6f44f3a
Compare
7 tasks
FusedCodecPipeline.supports_partial_decode/supports_partial_encode passed require_no_aa_bb=False, unlike BatchedCodecPipeline (True). With an outer array-array or bytes-bytes codec around a sharding serializer (e.g. compressors=[GzipCodec()], or filters=[TransposeCodec()]), the fused pipeline's partial read/write branches called ShardingCodec's partial sync methods directly on the raw stored value, skipping those outer codecs entirely. That wrote non-conforming bytes for an outer BB codec (unreadable by BatchedCodecPipeline or any conforming reader) and silently produced wrong data for an outer AA codec. Pass require_no_aa_bb=True in both fused properties so these chains fall through to the full-chunk fused path instead, matching batched behavior. Adds cross-pipeline parity coverage (full and partial read/write) for sharding with an outer compressor and with an outer transpose filter, and removes the "known limitation" exclusion that previously kept the sharding+compressor case out of the nested-sharding parity matrix. Assisted-by: ClaudeCode:claude-sonnet-5
d-v-b
force-pushed
the
fix/fused-partial-aa-bb
branch
from
July 29, 2026 14:32
6f44f3a to
3afccd2
Compare
…partial methods (zarr-developers#4201) The partial dispatch in FusedCodecPipeline.read_sync/write_sync asserted the private _decode_partial_sync/_encode_partial_sync methods, which only ShardingCodec implements. A codec advertising the public partial mixins (ArrayBytesCodecPartialDecodeMixin/-EncodeMixin) with only the documented async partial methods died with a bare AssertionError — or, under python -O, an AttributeError mid-IO. The asserts are now capability gates: codecs without the sync partial methods take the full-chunk sync path instead. The related crash for sharded arrays with async-only inner codecs is fixed separately in zarr-developers#4179. Assisted-by: ClaudeCode:claude-fable-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 🤖
Fixes the most severe finding from the PR zarr-developers#3885 audit. Only the opt-in
FusedCodecPipelineis affected.FusedCodecPipeline.supports_partial_decode/supports_partial_encodepassedrequire_no_aa_bb=False(whereBatchedCodecPipelinepassesTrue), so the partial-shard paths called the sharding codec's partial methods directly on the raw stored value — never applying codecs outside the sharding codec:serializer=sharding+ outergzipcompressor: fused wrote an un-gzipped shard blob while the array metadata declares gzip — unreadable by any conforming reader (BadGzipFilefrom batched; checksum mismatch when fused read batched-written data). Fused→fused round-trips passed, masking the corruption.filters=[transpose]+ sharding: silent wrong data in both cross-pipeline directions.The fix passes
require_no_aa_bb=Truein both properties; chains with outer AA/BB codecs now fall through to the full-chunk fused paths, which apply the whole codec chain correctly.Tests: the parity-matrix exclusion whose comment mislabeled this bug "a known limitation, not a pipeline-parity bug" is removed (gzip config re-enabled), and a new parametrized test covers outer-gzip and outer-transpose in both write/read directions (batched→fused and fused→batched) including a region write and region read. 15 of the new/re-enabled cases fail without the two-line fix (independently re-verified by the task reviewer via revert/rerun); 629 passed / 2 skipped on the verification suites plus a 2393-test broader sweep with no regressions.
🤖 Generated with Claude Code