Add unified JSON metadata validation via msgspec (#3285)

[vup903]

Summary

This PR consolidates zarr's scattered per-field JSON metadata validation
(parse_* helpers, ~42 of them across src/zarr) for #3285.

JSON type coercion now goes through msgspec.convert (Literal membership,
int/bool strictness, list-to-tuple, TypedDict with NotRequired), which
covers most of what the old helpers did by hand. A small fallback
(zarr.core.json_parse.validate_json_value) handles the two things msgspec
can't: the recursive JSON / JSONValue type alias (msgspec rejects it at
schema-build time) and PEP 728 extra_items= extension fields (msgspec
silently drops them). The registry/dtype/numcodec-bound parsers (parse_codecs,
parse_chunk_grid, parse_filters, dtype.from_json_scalar, …) stay
hand-written since they need runtime registry lookups msgspec can't do.

zarr.core.json_parse.parse_field(data, type_, field, error=...) wraps
convert and adds field context to the error message, so every migrated
per-field parser is now a one-liner.

What changed

• src/zarr/core/json_parse.py — rewritten around msgspec.convert:
  convert (field-agnostic type coercion), parse_field (adds field context to
  the error, using the caller's exception type), validate_json_value
  (recursive JSON validation with an explicit nesting-depth limit).
• Migrated helpers (now one-liners over parse_field/convert):
  parse_name, parse_order, parse_bool (common.py), parse_indexing_order
  (config.py), parse_separator (chunk_key_encodings.py), parse_zarr_format
  / parse_node_type (group.py), parse_zarr_format (metadata/v2.py),
  parse_zarr_format / parse_node_type_array (metadata/v3.py),
  parse_typesize / parse_clevel / parse_blocksize (codecs/blosc.py),
  parse_gzip_level (codecs/gzip.py), parse_zstd_level / parse_checksum
  (codecs/zstd.py). Every migrated helper keeps its original exception type
  and message.
• ArrayV3Metadata.from_dict validates attributes through
  validate_json_value.
• Bugfix: parse_storage_transformers used to call len(tuple(data)) and
  then return the original data, which silently exhausted a one-shot iterable
  and could return a non-tuple typed as one. Now materializes once into a tuple.
• pyproject.toml — added msgspec>=0.19 as a dependency (and pinned in the
  min_deps test env).
• tests/test_json_parse.py — tests for convert, parse_field,
  validate_json_value (including the nesting-depth limit), and a regression
  test for the parse_storage_transformers fix.

Net effect

json_parse.py goes from 291 to ~90 lines; test_json_parse.py from 479 to
~130. Net ~550 lines removed across the change. Two latent bugs caught: no
bound on JSON nesting depth (stack-exhaustion risk on adversarial attributes),
and the parse_storage_transformers iterator-exhaustion bug above.

Testing

• Full suites for the affected modules (test_json_parse, test_metadata,
  test_common, test_config, test_codecs, test_group) plus the test_array
  / test_api integration suites: all passing.
• uv run --frozen mypy: clean (189 source files).
• ruff check / ruff format --check: clean.

History

This started as a hand-written parse_json runtime type checker (the original
draft). @chuckwondo suggested msgspec could replace most of that logic, and
@d-v-b asked whether it could handle the real zarr-metadata types; a spike
confirmed msgspec covers the JSON-shaped cases but not JSONValue or
extra_items=. @d-v-b's guidance was not to require 100% coverage from one
tool, and to optimize for less code + catching bugs — this PR is the resulting
full migration, plus a follow-up from his review to encapsulate the
convert-then-re-raise pattern into parse_field.

Closes #3285.

[codecov]

Codecov Report

❌ Patch coverage is 97.72727% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.73%. Comparing base (6876120) to head (c596b68).

Files with missing lines	Patch %	Lines
src/zarr/codecs/blosc.py	90.90%	1 Missing ⚠️
src/zarr/codecs/zstd.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4063      +/-   ##
==========================================
+ Coverage   93.62%   93.73%   +0.11%     
==========================================
  Files          90       91       +1     
  Lines       11936    11960      +24     
==========================================
+ Hits        11175    11211      +36     
+ Misses        761      749      -12     

Files with missing lines	Coverage Δ
src/zarr/codecs/gzip.py	95.12% <100.00%> (+2.43%)	⬆️
src/zarr/core/chunk_key_encodings.py	92.53% <100.00%> (+1.36%)	⬆️
src/zarr/core/common.py	89.94% <100.00%> (+1.25%)	⬆️
src/zarr/core/config.py	100.00% <100.00%> (ø)
src/zarr/core/group.py	95.40% <100.00%> (+0.20%)	⬆️
src/zarr/core/json_parse.py	100.00% <100.00%> (ø)
src/zarr/core/metadata/v2.py	89.44% <100.00%> (ø)
src/zarr/core/metadata/v3.py	94.15% <100.00%> (+0.20%)	⬆️
src/zarr/codecs/blosc.py	97.65% <90.90%> (+2.34%)	⬆️
src/zarr/codecs/zstd.py	94.44% <87.50%> (+3.70%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vup903]

Hi @d-v-b, I put up a draft so you can see where I'm heading before I take it further.

So far it adds src/zarr/core/json_parse.py with a single parse_json(value, type_annotation) function. It validates a JSON-decoded value against a type annotation and returns assignable data (coercing sequences to tuples), or raises a clear error. I kept the scope to the JSON types listed in the issue: primitives, Literal, unions/Optional, tuple, Sequence/list, Mapping/dict, and TypedDict. There's a test file (tests/test_json_parse.py) with 94 cases covering each category, including the bool/int edge case and NotRequired under from __future__ import annotations.

On the de-dup side, I've moved 16 of the scattered parse_* helpers over to parse_json already. I split it into three small batches to keep each one easy to review: the pilot (parse_order, parse_bool, parse_zarr_format), the literal ones (parse_indexing_order, parse_node_type, parse_node_type_array, parse_separator, the zarr_format variants, parse_name), and the codec primitives (parse_clevel, parse_blocksize, parse_typesize, parse_gzip_level, parse_zstd_level, parse_checksum). Public signatures and the observable behavior (exception type and message) stay the same in every case.

Before I migrate the rest, a few things I'd like your read on:

1. Is src/zarr/core/json_parse.py a reasonable home for this, or would you rather it live somewhere else?
2. parse_json raises plain ValueError/TypeError, and the field-specific callers re-wrap into their own error (like MetadataValidationError). Does that split feel right to you?
3. For TypedDict, I resolve hints with get_type_hints(..., include_extras=True) and read Required/NotRequired plus __total__ instead of __required_keys__, so a class-syntax NotRequired still works when annotations are stringized. Open to doing it differently if you have a preference.
4. parse_json(x, int) rejects bool (since bool is an int subclass), where the old isinstance check let it through. I went with the stricter behavior because nothing relied on passing a bool there, but happy to revert if you'd rather keep it loose.

I'm holding off on the sequence/mapping/union helpers that dispatch to registries or factory methods until I hear back on the above.

One heads up on CI: the failing Test jobs are a pre-existing collection error in tests/test_group.py (duplicate parametrization of 'store') and tests/test_docs.py under the newer pytest. It's already on main and isn't from this change. Lint and pre-commit.ci are passing.

Let me know what you think on the four questions and I'll keep going.

[chuckwondo]

@vup903, if we are willing to add msgspec as a (very small) dependency, I believe we can eliminate the vast majority of this code, as msgspec.convert handles nearly all (if not all) of this logic. I might be off, as I have not thoroughly looked through the entirety of the affected code, but I'd like to hear from @d-v-b on this point.

[d-v-b]

nearly all of the types we need are now defined in zarr-metadata, so we should investigate if msgspec can handle all of those types correctly. zarr-metadata itself uses pydantic for its test suite, and I think i ended up choosing that after tryng msgspec and running into some issues, but I don't recall what they were. Maybe something to do with tuples? If its helpful, we could widen the JSON collection types in zarr-metadata to Sequence instead of tuple.

[vup903]

Thanks @chuckwondo, that's a good idea. I spent some time poking at msgspec.convert against these types to see how far it gets, and wanted to share what I found (msgspec 0.21.1, Python 3.12).

The good news is that most of the categories work out of the box, including the one you were worried about, @d-v-b:

• tuple coercion is fine now. msgspec.convert([1, 2, 3], tuple[int, ...]) returns (1, 2, 3), fixed-length tuples work, tuple[str | None, ...] works, and a wrong-length array is rejected. So whatever the old tuple issue was, I couldn't reproduce it on the current version.
• bool vs int is strict in the way we want: True is rejected for int and 1 is rejected for bool.
• Literal, Mapping[str, T], and TypedDict with NotRequired all behave correctly, including under from __future__ import annotations.

The two things that don't work are both load-bearing for the real zarr-metadata types, though:

1. The recursive JSONValue alias. msgspec.convert(doc, ArrayMetadataV3) fails with TypeError: Type 'JSONValue' is not supported. JSONValue is a recursive TypeAliasType (int | float | bool | None | str | list[JSONValue] | tuple[JSONValue, ...] | Mapping[str, JSONValue]), and msgspec doesn't accept it. Since fill_value, attributes, and the various config mappings are all typed as JSONValue, this blocks converting the actual array/group documents.

2. extra_items= (PEP 728). msgspec ignores it. With a TypedDict(..., extra_items=int), converting {"name": "x", "k": 5} silently drops k, and it doesn't even flag {"name": "x", "k": "not_an_int"} as invalid. ArrayMetadataV3 and GroupMetadataV3 use extra_items=ExtensionFieldV3 to validate v3 extension fields, so this matters for spec-correct validation of extension keys.

So my read is that msgspec handles the leaf/primitive/tuple cases really well, but it can't currently validate the two zarr-specific constructs (JSONValue and extra_items=) end to end. A couple of directions from here:

• If you'd be open to it, we could keep a thin hand-written layer just for the parts msgspec can't do (JSONValue and extra_items=) and lean on msgspec.convert for everything else. That would still delete most of the per-field parse_* code.
• Or, if widening the zarr-metadata collection types to Sequence (as you mentioned) also comes with dropping extra_items= / reshaping JSONValue into something msgspec understands, msgspec might cover the whole surface.

Happy to prototype whichever direction you prefer. What's your instinct here?

[d-v-b]

The recursive JSONValue alias. msgspec.convert(doc, ArrayMetadataV3) fails with TypeError: Type 'JSONValue' is not supported. JSONValue is a recursive TypeAliasType (int | float | bool | None | str | list[JSONValue] | tuple[JSONValue, ...] | Mapping[str, JSONValue]), and msgspec doesn't accept it. Since fill_value, attributes, and the various config mappings are all typed as JSONValue, this blocks converting the actual array/group documents.

extra_items= (PEP 728). msgspec ignores it. With a TypedDict(..., extra_items=int), converting {"name": "x", "k": 5} silently drops k, and it doesn't even flag {"name": "x", "k": "not_an_int"} as invalid. ArrayMetadataV3 and GroupMetadataV3 use extra_items=ExtensionFieldV3 to validate v3 extension fields, so this matters for spec-correct validation of extension keys.

these are both pretty important features! it's too bad that msgspec doesn't support them. but i don't think we need to reach 100% with just 1 tool. I would be interested in seeing how far we could get with msgspec, even if it doesn't get us all the way. The metric here should be increasing code simplicity (removing code) + catching bugs.

[vup903]

Pushed a full version of this. Everything msgspec can handle goes through
msgspec.convert now, and I kept a small fallback just for the bits it can't,
the recursive JSON values and the extra_items= extension fields. The
registry/dtype parsers (codecs, chunk grids, dtypes) I left as they are, since
those need runtime lookups.

It comes out to around 555 fewer lines, and it shook out a couple of latent bugs
on the way: nothing was capping JSON nesting depth, and
parse_storage_transformers was exhausting a one-shot iterable. Added msgspec as
a dependency and pinned it in min_deps. Everything's green.

Have a look whenever you get a chance, and let me know if you'd rather tweak the
version floor or move the fallback somewhere else.

[d-v-b]

I feel like this pattern: try cast(convert(...)), handle exception with error should be encapsulated in a reusable routine

[d-v-b]

we probably dont want that reusable routine to know the field name being validated. So we probably want to core validation routine to emit ValueError("Expected instance of $TYPE, got $data), and the caller should re-raise with a separate exception that raises ValueError("Failed to parse input for $FIELD) from the type-specific exception

[vup903]

Good idea, thanks. I moved it into a small parse_field helper like you
suggested: convert stays field-agnostic and just raises ValueError("Expected instance of {type}, got {data}"), and parse_field adds the field context
(Failed to parse input for {field}) with whatever exception type the caller
needs. Pushed it.

[vup903]

Marked this ready for review since the msgspec migration is complete and the
parse_field change from your last review is in. No rush, just flagging it's in
a reviewable state now. Happy to adjust anything else.