Refactor union dispatch to use reservation-based target assignment

[DZakh]

Summary

Refactors the union decoder to use a three-tier reservation algorithm for assigning source variants to target variants when compiling source → targetUnion. This improves correctness by preventing unreachable variants and enables better code generation through per-case .to application.

Key Changes

• New unionReservable function: Identifies variants that can participate in reservation (primitives, literals, null/undefined/NaN with no transformations).

• New unionAssignTargets function: Implements three-tier target assignment:

  • Tier 1: Same-type match with exact const preferred (reserved, exclusive)
  • Tier 2: Nullish bridge to opposite null/undefined (reserved, exclusive)
  • Tier 3: Remaining reservable variants coerce over free targets; non-reservable variants keep full target
  • Variants with no target assigned map to never and fail at runtime

• New helper functions: unionTypeValidationSchema, unionPrioritizeConst, unionActiveKey, unionIsPassthrough, unionRefinerAttacher, unionFail — extracted from inline union decoder logic for clarity and reusability.

• Refactored unionDecoder: Now delegates to unionEmit for the dispatch generation, using unionIsPassthrough to detect when input can be returned unchanged.

• Per-case .to application: When a union has a .to transformation and active key narrowing isn't possible, each variant's schema is updated with its assigned target and union refiners are attached per-case via unionRefinerAttacher.

• Code formatting: Fixed indentation and formatting throughout the union decoder section and related functions to match project conventions.

Implementation Details

The reservation algorithm ensures that when coercing source → targetUnion:

• A string source variant claims the string target variant (tier 1), leaving unit/None unreachable
• A null source can bridge to undefined target if no same-type match exists (tier 2)
• Structural variants (objects, arrays, instances, refs, nested unions) and transforming variants keep the full target and dispatch at runtime (tier 3)

This prevents silent data loss and makes union coercion semantics explicit and predictable. The refactored code is more maintainable with extracted helper functions that can be tested and reasoned about independently.

https://claude.ai/code/session_01CuMuwLb7tufLms3U3Gs3Kv

Summary by CodeRabbit

• Documentation

  • Clarified union conversion rules, including direct-match priority, nullish bridging, target reservation, and coercion behavior.
  • Added examples explaining exclusive matching and how to opt into cross-type transformations.

• Behavior Changes

  • Union conversions now prevent unmatched source variants from reusing targets claimed by direct matches.
  • Missing required fields and invalid "undefined" values now produce clear validation errors.
  • Improved synchronous error reporting for invalid union branches.

• Refactor

  • Improved internal union dispatch and shaped-schema processing without changing the public API.

[coderabbitai]

📝 Walkthrough

Walkthrough

Union conversion now reserves direct and nullish target cases, separates coercive fallback behavior, and updates generated decoder control flow. Documentation and tests reflect the new exclusivity rules, while shaped-schema helpers and internal formatting are reorganized.

Changes

Union conversion behavior

Layer / File(s)	Summary
Union target reservation contract docs/*-usage.md, packages/sury/src/Sury.res.mjs, packages/sury/tests/S_to*.res	Documents and tests define exclusive direct matches, reservable nullish bridges, non-reserving coercions, and updated failure behavior.
Union decoder generation and internal reflow packages/sury/src/Sury.res.mjs, packages/sury/src/Sury.res	Union decoder generation adds target assignment, passthrough detection, discriminant handling, and centralized failures; ReScript internals are primarily reformatted.
Shaped-schema traversal and parser outputs packages/sury/src/Sury.res.mjs	Shaped definitions, parser outputs, serializer outputs, and helper placement are reorganized around recursive traversal.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SourceUnion
  participant unionAssignTargets
  participant unionDecoder
  participant TargetSchema
  SourceUnion->>unionAssignTargets: provide source and target cases
  unionAssignTargets->>TargetSchema: reserve direct or nullish targets
  unionAssignTargets-->>unionDecoder: return target assignments
  unionDecoder->>TargetSchema: decode assigned cases

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: union dispatch was refactored around reservation-based target assignment.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/great-euler-12p4mn

⚔️ Resolve merge conflicts

• Resolve merge conflict in branch claude/great-euler-12p4mn

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/sury/src/Sury.res`:
- Around line 3122-3142: Update unionIsSelfDecodeNoop to require both
schema.refiner and schema.inputRefiner to be absent, alongside the existing
passthrough conditions. This ensures unionIsPassthrough never returns refined
input unchanged and preserves required user refiner calls.
- Around line 3493-3524: Update unionRefinerAttacher’s attach caching so it does
not reuse complete refiner check arrays across union cases. Regenerate
fn(~input) for each case-specific val, while caching only predicate data that is
independent of the case schema, expected type, path, and variable allocation;
preserve the existing merge behavior for current and newly generated checks.
- Around line 3282-3311: Update the tier-2 nullish bridge logic in the pass
containing `claim` so a source variant may claim the opposite nullish target
only when `unionReservable` approves it. Apply this check before selecting or
reserving a target, while preserving the existing null/undefined pairing and
reservation behavior for eligible variants.
- Around line 3253-3274: The fallback matching loop in the union reservation
pass must not reserve a mismatched literal target. In the `chosen.contents ===
-1` fallback within the `unionReservable` handling, require the candidate target
to be non-literal while retaining the `unionToKey` match; leave exact constant
matching in the preceding pass unchanged so broad targets remain available for
unmatched source literals.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad0dcade-7073-4370-80ef-271f5e04cb6e

📥 Commits

Reviewing files that changed from the base of the PR and between 7ee36f0 and f4da00b.

📒 Files selected for processing (6)

• docs/js-usage.md
• docs/rescript-usage.md
• packages/sury/src/Sury.res
• packages/sury/src/Sury.res.mjs
• packages/sury/tests/S_to_dictToObject_test.res
• packages/sury/tests/S_to_test.res

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not treat refined unions as passthroughs.

unionIsSelfDecodeNoop ignores refiner and inputRefiner. Consequently, unionIsPassthrough can return the input unchanged for a schema that still requires custom validation, silently skipping those refiners. Require both fields to be absent before classifying a schema as a self-decode noop.

As per coding guidelines: “Never skip this call when user refiners must be preserved.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sury/src/Sury.res` around lines 3122 - 3142, Update
unionIsSelfDecodeNoop to require both schema.refiner and schema.inputRefiner to
be absent, alongside the existing passthrough conditions. This ensures
unionIsPassthrough never returns refined input unchanged and preserves required
user refiner calls.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not reserve mismatched literal targets.

The fallback tag match lets source literal "a" reserve target literal "b" when no exact constant matches. If a broad string target is also available, "a" is assigned to "b" and fails at runtime instead of using the broad target.

Restrict this fallback to non-literal targets; exact literal matches are already handled by the preceding pass.

Suggested fix

-            if unionToKey(targetCases->Array.getUnsafe(ti)) === sKey {
+            let target = targetCases->Array.getUnsafe(ti)
+            if unionToKey(target) === sKey && !target->isLiteral {
               chosen := ti
             }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Pass 1: tier 1 — same-type match (exact const preferred), reserves it.
	for si in 0 to sourceLen - 1 {
	let s = sources->Array.getUnsafe(si)
	if unionReservable(s) {
	let sKey = unionToKey(s)
	let chosen = ref(-1)
	for ti in 0 to targetLen - 1 {
	if chosen.contents === -1 && !(reserved->Array.getUnsafe(ti)) {
	let t = targetCases->Array.getUnsafe(ti)
	if unionToKey(t) === sKey && t.const === s.const {
	chosen := ti
	}
	}
	}
	if chosen.contents === -1 {
	for ti in 0 to targetLen - 1 {
	if chosen.contents === -1 && !(reserved->Array.getUnsafe(ti)) {
	if unionToKey(targetCases->Array.getUnsafe(ti)) === sKey {
	chosen := ti
	}
	}
	}
	// Pass 1: tier 1 — same-type match (exact const preferred), reserves it.
	for si in 0 to sourceLen - 1 {
	let s = sources->Array.getUnsafe(si)
	if unionReservable(s) {
	let sKey = unionToKey(s)
	let chosen = ref(-1)
	for ti in 0 to targetLen - 1 {
	if chosen.contents === -1 && !(reserved->Array.getUnsafe(ti)) {
	let t = targetCases->Array.getUnsafe(ti)
	if unionToKey(t) === sKey && t.const === s.const {
	chosen := ti
	}
	}
	}
	if chosen.contents === -1 {
	for ti in 0 to targetLen - 1 {
	if chosen.contents === -1 && !(reserved->Array.getUnsafe(ti)) {
	let target = targetCases->Array.getUnsafe(ti)
	if unionToKey(target) === sKey && !target->isLiteral {
	chosen := ti
	}
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sury/src/Sury.res` around lines 3253 - 3274, The fallback matching
loop in the union reservation pass must not reserve a mismatched literal target.
In the `chosen.contents === -1` fallback within the `unionReservable` handling,
require the candidate target to be non-literal while retaining the `unionToKey`
match; leave exact constant matching in the preceding pass unchanged so broad
targets remain available for unmatched source literals.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Only reservable variants may claim nullish bridges.

This pass does not call unionReservable, so a transformed null/undefined variant with .to, parser, or serializer can reserve the opposite nullish target. That violates the reservation contract and can force the transformed case into an incompatible target.

Suggested fix

     | None =>
       let sTag = (sources->Array.getUnsafe(si)).tag
-      let oppositeTag = if sTag === nullTag {
+      if unionReservable(sources->Array.getUnsafe(si)) {
+        let oppositeTag = if sTag === nullTag {
           undefinedTag
-      } else if sTag === undefinedTag {
+        } else if sTag === undefinedTag {
           nullTag
-      } else {
+        } else {
           sTag
-      }
-      if oppositeTag !== sTag {
+        }
+        if oppositeTag !== sTag {
           ...
+        }
       }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Pass 2: tier 2 — nullish bridge to the opposite nullish target, reserves it.
	for si in 0 to sourceLen - 1 {
	switch assigned->Array.getUnsafe(si) {
	| Some(_) => ()
	| None =>
	let sTag = (sources->Array.getUnsafe(si)).tag
	let oppositeTag = if sTag === nullTag {
	undefinedTag
	} else if sTag === undefinedTag {
	nullTag
	} else {
	sTag
	}
	if oppositeTag !== sTag {
	let chosen = ref(-1)
	for ti in 0 to targetLen - 1 {
	if (
	chosen.contents === -1 &&
	!(reserved->Array.getUnsafe(ti)) &&
	(targetCases->Array.getUnsafe(ti)).tag === oppositeTag
	) {
	chosen := ti
	}
	}
	if chosen.contents !== -1 {
	claim(si, chosen.contents)
	}
	}
	}
	}
	// Pass 2: tier 2 — nullish bridge to the opposite nullish target, reserves it.
	for si in 0 to sourceLen - 1 {
	switch assigned->Array.getUnsafe(si) {
	| Some(_) => ()
	| None =>
	let sTag = (sources->Array.getUnsafe(si)).tag
	if unionReservable(sources->Array.getUnsafe(si)) {
	let oppositeTag = if sTag === nullTag {
	undefinedTag
	} else if sTag === undefinedTag {
	nullTag
	} else {
	sTag
	}
	if oppositeTag !== sTag {
	let chosen = ref(-1)
	for ti in 0 to targetLen - 1 {
	if (
	chosen.contents === -1 &&
	!(reserved->Array.getUnsafe(ti)) &&
	(targetCases->Array.getUnsafe(ti)).tag === oppositeTag
	) {
	chosen := ti
	}
	}
	if chosen.contents !== -1 {
	claim(si, chosen.contents)
	}
	}
	}
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sury/src/Sury.res` around lines 3282 - 3311, Update the tier-2
nullish bridge logic in the pass containing `claim` so a source variant may
claim the opposite nullish target only when `unionReservable` approves it. Apply
this check before selecting or reserving a target, while preserving the existing
null/undefined pairing and reservation behavior for eligible variants.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not cache complete refiner checks across cases.

fn(~input) receives a val, so the generated condition and failure builder may depend on that case’s schema, expected type, path, or variable allocation. Caching the entire check array from the first case reuses those val-specific checks for every later case. Cache only reusable embedded predicate data, or regenerate the checks per case.

As per coding guidelines: “Treat val as the compile-time view of a runtime value and preserve its core fields.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sury/src/Sury.res` around lines 3493 - 3524, Update
unionRefinerAttacher’s attach caching so it does not reuse complete refiner
check arrays across union cases. Regenerate fn(~input) for each case-specific
val, while caching only predicate data that is independent of the case schema,
expected type, path, and variable allocation; preserve the existing merge
behavior for current and newly generated checks.

Source: Coding guidelines