Run a metadata coercer before a producing Annotated base#215
Conversation
A dataclass field's Annotated base is pinned at position 0 by Python and is also the field's static type, so metadata cannot be placed before it. probatio ordered the base relative to its metadata by the base's role: a plain type asserts and runs last (a coercer in the metadata runs first, the type confirms, ADR-008), but a producing base (an enum, a nested schema, a container) runs first. That left no way to reshape a raw value before a producing base coerced it, so a Map that folds an enum sentinel to None never ran: the enum rejected the sentinel first. Extend the ADR-008 rule the other way: when the metadata itself carries a coercer (a Coerce or Map, including one wrapped in Maybe or a union), the coercer runs first and the producing base confirms the result. Only the coercion markers flip the order; an asserting constraint like In, or a bare callable that cannot be told apart from a plain check, keeps the base-first order and sees the produced value. This keeps an enum-typed field typed as the enum while a Map cleans its sentinels ahead of it, so a from_dict override is no longer needed for that pattern.
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR changes how ChangesAnnotated Metadata Ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0ef821d6-164c-4c19-b340-c0d105dfbf8a
📒 Files selected for processing (4)
docs/src/content/docs/guides/dataclasses.mddocs/src/content/docs/recipes/cookbook.mdsrc/probatio/dataclass_schema.pytests/test_dataclass_schema.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Fuzz fuzz_from_json_schema
- GitHub Check: Fuzz fuzz_is_catastrophic
🧰 Additional context used
📓 Path-based instructions (3)
src/probatio/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/probatio/**/*.py: Keep the public API aligned with voluptuous, and do not rename or re-signature public names without a documented reason.
Every public function and class must have a docstring.
Do not copy code from voluptuous; implement behavior cleanly and independently while matching behavior rather than source.
Files:
src/probatio/dataclass_schema.py
{src/probatio,tests}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Match the surrounding code style and prefer small, well-named functions.
Files:
src/probatio/dataclass_schema.pytests/test_dataclass_schema.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Every test must have a one-line docstring describing what it verifies.
Files:
tests/test_dataclass_schema.py
🔇 Additional comments (9)
src/probatio/dataclass_schema.py (2)
179-204: LGTM!
251-258: LGTM!tests/test_dataclass_schema.py (4)
543-548: LGTM!
550-554: LGTM!
557-561: LGTM!
522-563: 🎯 Functional Correctness
AnyOfis alreadyAnyhere; no change needed.tests/test_dataclass_schema.pyimportsAny as AnyOf, so_metadata_coercesalready matches it viaAny.> Likely an incorrect or invalid review comment.docs/src/content/docs/guides/dataclasses.md (1)
145-149: LGTM!docs/src/content/docs/recipes/cookbook.md (2)
168-171: LGTM!
191-194: LGTM!
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR extends the Annotated metadata ordering rule in DataclassSchema so that a coercer in the metadata (Coerce or Map, including one wrapped in a Maybe/union) runs before a producing base such as an Enum. This enables preprocessing a raw value (e.g. folding an enum sentinel to None) before the producing base coerces it, while keeping the field typed as the enum.
Changes:
- Add
_metadata_coerceshelper and use it in_annotation_to_schemato flip the base/metadata order when the metadata coerces. - Add tests covering
Mapsentinel,Maybe(Coerce(...)), and union-wrapped coercer cases. - Document the new behavior in the dataclasses guide and cookbook.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/probatio/dataclass_schema.py | Introduces _metadata_coerces and flips ordering so a metadata coercer runs before a producing base. |
| tests/test_dataclass_schema.py | Adds dataclasses and tests validating the new coercer-first ordering. |
| docs/src/content/docs/recipes/cookbook.md | Adds a dataclass-field example for the enum-sentinel Map case. |
| docs/src/content/docs/guides/dataclasses.md | Documents that a metadata coercer runs ahead of a producing base. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The block used Map, PASSTHROUGH, and VehicleType from an earlier block on the page. It runs on the docs site (blocks share a namespace) but the cookbook's promise is that a single block copies and runs on its own, so import the coercers and define the enum in the block.
Breaking change
None in the common case, but it is a deliberate ordering change worth stating. When a dataclass field's
Annotatedmetadata contains a coercer (CoerceorMap, including one wrapped inMaybeor a union) and the base is a producing type (an enum, a nested dataclass, a container), the coercer now runs before the base instead of after it. Metadata that only asserts (likeIn) and bare callables are unchanged, and a plain-type base was already metadata-first, so no existing schema of those shapes changes. The full suite and the compiled-vs-interpreted parity lane both stay green.Proposed change
A dataclass field's
Annotatedbase is pinned at position 0 by Python and is also the field's static type, so you cannot place metadata before it. probatio ordered the base relative to its metadata by the base's role: a plain type asserts and runs last (a coercer in the metadata runs first, the type confirms, ADR-008), but a producing base (an enum, a nested schema, a container) runs first. That left no way to reshape a raw value before a producing base coerced it. The concrete case: aMapthat folds an enum sentinel ("N.v.t.") toNonenever ran, because the enum rejected the sentinel first.This extends the ADR-008 rule the other way: when the metadata itself carries a coercer, the coercer runs first and the producing base confirms the result. Only the coercion markers flip the order; an asserting constraint like
In, or a bare callable that cannot be told apart from a plain check, keeps the base-first order and sees the produced value.So an enum-typed field stays typed as the enum while a
Mapcleans its sentinels ahead of it:This came out of a real migration (an RDW client moving off mashumaro): a scraped government dataset sends sentinel strings for missing enum values, and the only way to null them per field previously was a hand-written
from_dictoverride or dropping the field tostr. With this change the annotation-only form works and keeps the enum type.Follows the
Mappassthrough default (#214), which is the other half of the same use case.Type of change
Additional information
Checklist
uv run --no-sync just testpasses locally. A pull request cannot be merged unless CI is green.uv run --no-sync just lintpasses.uv run --no-sync just typecheckpasses.