From be8f65d6d10dda97592d31597b8dee6a5df89bfc Mon Sep 17 00:00:00 2001
From: David Stuebe <84335963+emfdavid@users.noreply.github.com>
Date: Thu, 16 Jul 2026 02:53:03 +0000
Subject: [PATCH] Fix bytes codec endian for big-endian structured dtypes

The byte order of the array-to-bytes codec was taken from dtype.byteorder,
which numpy reports as "|" for every structured dtype no matter what its
fields hold. Big-endian record data therefore fell through to the
little-endian default, and zarr faithfully decoded the referenced bytes in
the byte order it had been told -- the wrong one -- so virtual references to
a FITS BinTableHDU (e.g. an SDSS spectrum) yielded silently wrong values.

Read the byte order from the fields instead, looking through subarray fields
(whose order hides behind .base) and nested structs, and ignoring fields that
carry no byte order at all (single-byte numbers, strings) rather than letting
them imply little-endian. A structured dtype whose fields mix byte orders now
raises: a Zarr V3 array has a single bytes codec and cannot express it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 docs/about/releases.md            |  2 ++
 virtualizarr/codecs.py            | 39 ++++++++++++++++++++++++++++++-
 virtualizarr/tests/test_codecs.py | 37 +++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/docs/about/releases.md b/docs/about/releases.md
index a5dc07368..33774b933 100644
--- a/docs/about/releases.md
+++ b/docs/about/releases.md
@@ -16,6 +16,8 @@ Adds a `nrefs` accessor for counting virtual chunk references, a `mode` paramete
 ### Bug fixes
 - Fix parsing kerchunk references that use a **structured (record) dtype**, as produced for a FITS `BinTableHDU` (e.g. an SDSS spectrum). Such references round-trip the dtype through JSON as a list of `[name, format]` lists and encode the `fill_value` as base64 raw bytes; `from_kerchunk_refs` now coerces the field specs back to tuples before `np.dtype` and decodes the base64 `fill_value` into a structured scalar, instead of raising `TypeError`.
   By [David Stuebe](https://github.com/emfdavid).
+- Fix the bytes codec recording the wrong `endian` for a **big-endian structured dtype**, which made virtual references to big-endian record data (e.g. a FITS `BinTableHDU`) decode to silently wrong values. The byte order was read from `dtype.byteorder`, which numpy reports as `"|"` for any structured dtype whatever its fields hold, so such arrays fell through to the little-endian default; it is now read from the fields, looking through subarray and nested-struct fields and ignoring fields that carry no byte order (single-byte numbers, strings). A structured dtype whose fields mix byte orders now raises, since a Zarr V3 array has a single bytes codec and cannot express it.
+  By [David Stuebe](https://github.com/emfdavid).
 
 - Writing a virtual `DataTree` with `region` or `append_dim` (forwarded to each node) previously always failed with `ContainsGroupError`, because every group was unconditionally created; the existing groups are now opened instead.
   By [Aaron Spring](https://github.com/aaronspring).
diff --git a/virtualizarr/codecs.py b/virtualizarr/codecs.py
index d60f2fc12..623d83f50 100644
--- a/virtualizarr/codecs.py
+++ b/virtualizarr/codecs.py
@@ -77,6 +77,43 @@ def extract_codecs(
     return (arrayarray_codecs, arraybytes_codec, bytesbytes_codecs)
 
 
+def _byte_orders(dtype: np.dtype) -> set[str]:
+    """
+    Collect the byte-order characters of a dtype's multi-byte values.
+
+    ``dtype.byteorder`` describes only the dtype itself, and numpy reports ``"|"`` for every
+    structured dtype regardless of what its fields hold, so the fields are walked instead. A
+    subarray field (``("<f4", (3,))``) hides its byte order behind ``.base``, and a nested struct
+    hides it one level further down, hence the recursion. Values that have no byte order at all --
+    single-byte numbers, strings -- contribute nothing rather than counting as little-endian.
+    """
+    base = dtype.base  # see through a subarray field
+    if base.fields is None:
+        return set() if base.byteorder == "|" else {base.byteorder}
+    return set().union(*(_byte_orders(field[0]) for field in base.fields.values()))
+
+
+def _is_big_endian(dtype: np.dtype) -> bool:
+    """
+    Whether ``dtype``'s multi-byte values are stored big-endian.
+
+    Raises
+    ------
+    ValueError
+        If the dtype's fields mix byte orders, which a Zarr V3 array cannot express.
+    """
+    orders = _byte_orders(dtype)
+    if ">" not in orders:
+        return False
+    if len(orders) > 1:
+        raise ValueError(
+            f"Cannot represent {dtype} as a Zarr V3 array: its fields mix byte orders "
+            f"({sorted(orders)}). A Zarr V3 array has one bytes codec, so every multi-byte "
+            f"field must share a single byte order."
+        )
+    return True
+
+
 def convert_to_codec_pipeline(
     dtype: np.dtype,
     codecs: list[dict] | None = [],
@@ -106,7 +143,7 @@ def convert_to_codec_pipeline(
     arrayarray_codecs, arraybytes_codec, bytesbytes_codecs = extract_codecs(zarr_codecs)
 
     if arraybytes_codec is None:
-        if dtype.byteorder == ">":
+        if _is_big_endian(dtype):
             arraybytes_codec = BytesCodec(endian="big")
         else:
             arraybytes_codec = BytesCodec()
diff --git a/virtualizarr/tests/test_codecs.py b/virtualizarr/tests/test_codecs.py
index f87a2c3e8..6c2fef71d 100644
--- a/virtualizarr/tests/test_codecs.py
+++ b/virtualizarr/tests/test_codecs.py
@@ -122,6 +122,43 @@ def test_convert_to_codec_pipeline_scenarios(self, input_codecs, expected_pipeli
         result = convert_to_codec_pipeline(dtype=dtype, codecs=input_codecs)
         assert result == expected_pipeline
 
+    @pytest.mark.parametrize(
+        "dtype,expected_endian",
+        [
+            # Plain dtypes: numpy reports the byte order on the dtype itself.
+            (np.dtype(">i4"), "big"),
+            (np.dtype("<i4"), "little"),
+            # Structured dtypes: numpy reports byteorder "|" on the dtype whatever its
+            # fields say, so the byte order has to be read off the fields.
+            (np.dtype([("flux", ">f4"), ("ivar", ">f4")]), "big"),
+            (np.dtype([("flux", "<f4"), ("ivar", "<f4")]), "little"),
+            # Fields that carry no byte order (single-byte, strings) are not evidence of
+            # little-endian storage -- they are ignored.
+            (np.dtype([("flux", ">f4"), ("name", "S8")]), "big"),
+            (np.dtype([("flux", ">f4"), ("flag", "i1")]), "big"),
+            (np.dtype([("flag", "i1"), ("name", "S8")]), "little"),
+            # A subarray field hides its byte order behind ``.base``.
+            (np.dtype([("flux", ">f4", (3,))]), "big"),
+            # A nested struct hides it another level down.
+            (np.dtype([("coadd", [("flux", ">f4")])]), "big"),
+        ],
+    )
+    def test_byte_order_is_read_from_struct_fields(self, dtype, expected_endian):
+        """The bytes codec's endian must describe the *stored* byte order.
+
+        A FITS binary table arrives as a big-endian structured dtype; picking the codec from
+        ``dtype.byteorder`` (which is "|" for any struct) silently stored it as little-endian.
+        """
+        result = convert_to_codec_pipeline(dtype=dtype, codecs=None)
+        assert result.array_bytes_codec == BytesCodec(endian=expected_endian)
+
+    def test_mixed_byte_order_struct_raises(self):
+        """Zarr v3 gives an array a single bytes codec, so one array cannot store both."""
+        with pytest.raises(ValueError, match="single byte order"):
+            convert_to_codec_pipeline(
+                dtype=np.dtype([("big", ">f4"), ("little", "<f4")]), codecs=None
+            )
+
 
 class TestExtractCodecs:
     """Test the extract_codecs function."""