Skip to content

fix: FusedCodecPipeline must apply outer AA/BB codecs on partial paths - #4202

Merged
d-v-b merged 2 commits into
zarr-developers:mainfrom
d-v-b:fix/fused-partial-aa-bb
Jul 29, 2026
Merged

fix: FusedCodecPipeline must apply outer AA/BB codecs on partial paths#4202
d-v-b merged 2 commits into
zarr-developers:mainfrom
d-v-b:fix/fused-partial-aa-bb

Conversation

@d-v-b

@d-v-b d-v-b commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Fixes the fused codec pipeline to avoid data corruption caused by failing to avoid partial decode path for codec pipelines combining "container" serializers (sharding) flanked by array-array and / or bytes-bytes codecs.

Assisted-by: ClaudeCode:claude-sonnet-5

based on a claude-authored PR here: d-v-b#255

Summary

[Describe what this PR changes and why, in your own words.]

For reviewers

[What would you most value a second look at? What are you already confident in? For a refactor, say whether behavior is meant to be unchanged.]

Author attestation

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

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
d-v-b force-pushed the fix/fused-partial-aa-bb branch from 6f44f3a to 3afccd2 Compare July 29, 2026 14:32
@d-v-b
d-v-b marked this pull request as ready for review July 29, 2026 14:37
@d-v-b

d-v-b commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

self-merging, since this fixes a data corruption issue!

@d-v-b
d-v-b merged commit ba83236 into zarr-developers:main Jul 29, 2026
24 of 25 checks passed
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.90%. Comparing base (123268a) to head (8faa833).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4202      +/-   ##
==========================================
- Coverage   93.90%   93.90%   -0.01%     
==========================================
  Files          91       91              
  Lines       12672    12670       -2     
==========================================
- Hits        11900    11898       -2     
  Misses        772      772              
Files with missing lines Coverage Δ
src/zarr/core/codec_pipeline.py 96.20% <ø> (-0.02%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant