Skip to content

Feature/issue 511 type registry#972

Open
Dee66 wants to merge 2 commits intozio:mainfrom
Dee66:feature/issue-511-type-registry
Open

Feature/issue 511 type registry#972
Dee66 wants to merge 2 commits intozio:mainfrom
Dee66:feature/issue-511-type-registry

Conversation

@Dee66
Copy link
Copy Markdown

@Dee66 Dee66 commented Dec 24, 2025

Description

This PR addresses Issue #511 by enabling ExtensibleMetaSchema.materialize to utilize an explicit Map[TypeId, Schema[_]] registry. This allows for the successful reconstruction of typed schemas from ASTs even when those types are not part of the standard built-in instances.

Key Changes

  • Registry Integration: Updated materialize to prioritize lookups in the provided registry.
  • Type-Safe Fallback: Implemented the .collect { case ... } pattern in Sum and Product cases. This ensures that if the registry contains a type-mismatched schema, the system gracefully falls back to AST-based construction rather than failing or casting incorrectly.
  • Improved Resilience: Hardened DynamicValue reconstruction for enums by wrapping construction in Try to propagate meaningful DecodeErrors.
  • Recursive Support: Ensured proper propagation of both refs and registry through nested and recursive structures.

Verification

  • New Test Suite: Added Issue511Spec with 5 scenarios:
    • Recursive data types (Category/subCategories).
    • Custom registry prioritization for Products and Sums.
    • Graceful fallback for "poisoned" (type-mismatched) registry entries.
  • Regression Testing: All 849 existing tests passed (JVM).

/claim #511

@Dee66 Dee66 requested a review from a team as a code owner December 24, 2025 11:47
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Dec 24, 2025

CLA assistant check
All committers have signed the CLA.

@Dee66 Dee66 force-pushed the feature/issue-511-type-registry branch 3 times, most recently from baf0d6e to 738272a Compare December 24, 2025 18:30
@Dee66
Copy link
Copy Markdown
Author

Dee66 commented Dec 24, 2025

Status Update: All CI checks are now passing, including logic tests for #511 and binary compatibility (MiMa). The solution provides the necessary TypeRegistry support for AST materialization while maintaining backward compatibility through method overloading.

Ready for review.

@Dee66
Copy link
Copy Markdown
Author

Dee66 commented Mar 2, 2026

Please can this ticket be reviewed?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances ExtensibleMetaSchema.materialize to optionally use a caller-provided Map[TypeId, Schema[_]] registry, improving typed schema reconstruction from ASTs for non-built-in types (Issue #511), and hardens enum DynamicValue decoding.

Changes:

  • Added toSchema(registry: Map[TypeId, Schema[_]]) and threaded registry through ExtensibleMetaSchema.materialize so Products/Sums can prefer registry schemas safely.
  • Updated DynamicValue enum decoding to construct enum cases defensively and surface a meaningful DecodeError when construction throws.
  • Added Issue511Spec test coverage for registry-based reconstruction, nested products, enums, and “poisoned” registries.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
zio-schema/shared/src/main/scala/zio/schema/meta/ExtensibleMetaSchema.scala Adds registry-aware AST materialization and propagates registry through nested schema construction.
zio-schema/shared/src/main/scala/zio/schema/DynamicValue.scala Improves enum decoding by constructing enum values from decoded case payloads and mapping construction failures to DecodeError.
tests/shared/src/test/scala/zio/schema/Issue511Spec.scala Adds regression tests for Issue #511 scenarios including registry prioritization and poisoning fallback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/shared/src/test/scala/zio/schema/Issue511Spec.scala Outdated
Comment thread tests/shared/src/test/scala/zio/schema/Issue511Spec.scala Outdated
Comment thread zio-schema/shared/src/main/scala/zio/schema/DynamicValue.scala Outdated
Comment thread zio-schema/shared/src/main/scala/zio/schema/meta/ExtensibleMetaSchema.scala Outdated
@Dee66
Copy link
Copy Markdown
Author

Dee66 commented Mar 3, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

@Dee66 Dee66 force-pushed the feature/issue-511-type-registry branch from 9320d0a to 3c8fde2 Compare March 5, 2026 14:14
Allows DynamicValue to correctly reconstruct typed records and enums by looking up existing schemas in the provided TypeRegistry during MetaSchema materialization. This ensures recursive and complex types are resolved correctly from their AST representation.
@Dee66 Dee66 force-pushed the feature/issue-511-type-registry branch from 3c8fde2 to 0d189e5 Compare March 5, 2026 15:08
@Dee66
Copy link
Copy Markdown
Author

Dee66 commented Mar 5, 2026

@987Nabil code review suggestions are implemented, please review.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants