fix: address upstream review feedback on multi-type union fix

CHANGELOG.adoc

@@ -13,6 +13,9 @@
 
 == Unreleased changes (barbacane-dev fork)
 
+=== Tests / docs
+* tighten the multi-type `type` + subschemas regression fixture and code comment, per upstream review feedback on https://github.com/oxidecomputer/typify/pull/1001[oxidecomputer/typify#1001]: drop the `SingleTypeOneOfArrayBranch` case (whose contradictory schema caused codegen to emit an unreachable enum variant), trim verbose `$comment` rationales, and shorten the match-arm doc comment in `convert.rs`
+
 https://github.com/barbacane-dev/typify/compare/v1.0.0\...HEAD[Full list of commits]
 
 == 1.0.0 (released 2026-05-13)

typify-impl/src/convert.rs

@@ -464,12 +464,9 @@ impl TypeSpace {
                 extensions: _,
             } => self.convert_unknown_enum(type_name, original_schema, metadata, enum_values),
 
-            // Subschemas with no body validation and either no type or a
+            // Subschemas with no additional validation and either no type or a
             // single type. A multi-type (`Vec`) instance_type is deliberately
-            // excluded so that it flows through the merge arm below, which
-            // folds the type union into each subschema branch. Preserving the
-            // earlier behaviour for `None` / `Single` keeps existing tolerant
-            // handling of schemas whose outer type may conflict with a branch.
+            // excluded so that it flows through the merge arm below.
             SchemaObject {
                 metadata,
                 instance_type: None | Some(SingleOrVec::Single(_)),

typify/tests/schemas/type-array-with-subschemas.json

@@ -1,9 +1,8 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "$comment": "Regression coverage for issue #954: schemas with a multi-type `type: [...]` array on the same object as `oneOf` / `anyOf` / `allOf` / `not` previously discarded the `type` constraint (TODO at convert.rs wildcarded `instance_type`). Single-type + subschema cases must still pass through the earlier arm unchanged.",
+  "$comment": "Coverage for issue #954: multi-type `type: [...]` alongside oneOf/anyOf/allOf/not.",
   "definitions": {
     "TypeArrayOneOfItems": {
-      "$comment": "Canonical issue #954 shape. Each oneOf branch only constrains `items`, so the type union must be folded into every branch rather than dropped.",
       "type": [ "string", "number", "boolean", "array" ],
       "oneOf": [
         { "items": { "type": "string" } },
@@ -12,53 +11,33 @@
       ]
     },
     "TypeArrayAnyOfItems": {
-      "$comment": "Same shape as TypeArrayOneOfItems but using anyOf. anyOf travels through try_merge_with_each_subschema on a sibling path from oneOf; it should fold the type union the same way.",
       "type": [ "string", "number", "array" ],
       "anyOf": [
         { "items": { "type": "string" } },
         { "items": { "type": "number" } }
       ]
     },
     "TypeArrayAllOfRefinement": {
-      "$comment": "allOf is folded pairwise into the parent rather than producing branches. The type union must survive and the array-only constraints should apply when the Array variant is selected.",
       "type": [ "string", "array" ],
       "allOf": [
         { "items": { "type": "string" } },
         { "minItems": 1 }
       ]
     },
     "TypeArrayNotExclusion": {
-      "$comment": "not: object is redundant when the outer type union excludes object, but merging must not drop the type union when the not branch is applied.",
       "type": [ "string", "number", "array" ],
       "not": { "type": "object" }
     },
-    "SingleTypeOneOfArrayBranch": {
-      "$comment": "Regression guard (rust-collisions pattern). Outer singleton type + oneOf where one branch has a conflicting explicit type. This must continue to pass through the earlier arm (no merge), otherwise the array branch becomes unsatisfiable and is silently dropped.",
-      "type": "object",
-      "oneOf": [
-        {
-          "type": "object",
-          "properties": { "kind": { "type": "string" } },
-          "required": [ "kind" ]
-        },
-        {
-          "type": "array",
-          "items": { "type": "string" },
-          "minItems": 2,
-          "maxItems": 2
-        }
-      ]
-    },
     "TypeArrayOneOfExplicitArrayBranches": {
-      "$comment": "Case 7: each oneOf branch pins `type: array`, so the intersection with the outer type union must prune the non-array primitives. Only array variants should be emitted.",
+      "$comment": "Each oneOf branch pins `type: array`; the non-array variants from the outer union should be pruned.",
       "type": [ "string", "array" ],
       "oneOf": [
         { "type": "array", "items": { "type": "string" } },
         { "type": "array", "items": { "type": "number" } }
       ]
     },
     "TypeArrayPartiallyUnsatisfiableOneOf": {
-      "$comment": "Some oneOf branches conflict with the outer type union and should be dropped during merge; the surviving branch must carry the outer type union. The two eliminated branches use object/number which the outer `[string, array]` disallows.",
+      "$comment": "Two oneOf branches conflict with the outer type union and should be dropped during merge.",
       "type": [ "string", "array" ],
       "oneOf": [
         { "type": "object", "properties": { "name": { "type": "string" } } },
@@ -67,15 +46,15 @@
       ]
     },
     "TypeArrayFullyUnsatisfiableOneOf": {
-      "$comment": "Case 9: every branch conflicts with the outer type union, so `try_merge_with_each_subschema` returns empty and the schema resolves to never. Must emit an empty enum cleanly rather than panic.",
+      "$comment": "Every branch conflicts with the outer type union; must resolve cleanly rather than panic.",
       "type": [ "string", "number" ],
       "oneOf": [
         { "type": "array", "items": { "type": "string" } },
         { "type": "object", "properties": { "k": { "type": "string" } } }
       ]
     },
     "TypeArrayOneOfAndAllOf": {
-      "$comment": "Case 10: oneOf and allOf on the same object, both alongside a multi-type `type` array. Exercises the full merge path (allOf folded first, then oneOf fanned out) with the Vec instance_type flowing through the merge arm.",
+      "$comment": "oneOf and allOf on the same object, plus a multi-type `type` array.",
       "type": [ "string", "array" ],
       "allOf": [
         { "minLength": 1 }

typify/tests/schemas/type-array-with-subschemas.rs

@@ -25,56 +25,12 @@ pub mod error {
         }
     }
 }
-#[doc = "`SingleTypeOneOfArrayBranch`"]
-#[doc = r""]
-#[doc = r" <details><summary>JSON schema</summary>"]
-#[doc = r""]
-#[doc = r" ```json"]
-#[doc = "{"]
-#[doc = "  \"$comment\": \"Regression guard (rust-collisions pattern). Outer singleton type + oneOf where one branch has a conflicting explicit type. This must continue to pass through the earlier arm (no merge), otherwise the array branch becomes unsatisfiable and is silently dropped.\","]
-#[doc = "  \"oneOf\": ["]
-#[doc = "    {"]
-#[doc = "      \"properties\": {"]
-#[doc = "        \"kind\": {"]
-#[doc = "          \"type\": \"string\""]
-#[doc = "        }"]
-#[doc = "      },"]
-#[doc = "      \"required\": ["]
-#[doc = "        \"kind\""]
-#[doc = "      ],"]
-#[doc = "      \"type\": \"object\""]
-#[doc = "    },"]
-#[doc = "    {"]
-#[doc = "      \"items\": {"]
-#[doc = "        \"type\": \"string\""]
-#[doc = "      },"]
-#[doc = "      \"maxItems\": 2,"]
-#[doc = "      \"minItems\": 2,"]
-#[doc = "      \"type\": \"array\""]
-#[doc = "    }"]
-#[doc = "  ],"]
-#[doc = "  \"type\": \"object\""]
-#[doc = "}"]
-#[doc = r" ```"]
-#[doc = r" </details>"]
-#[derive(:: serde :: Deserialize, :: serde :: Serialize, Clone, Debug)]
-#[serde(untagged)]
-pub enum SingleTypeOneOfArrayBranch {
-    Object { kind: ::std::string::String },
-    Array([::std::string::String; 2usize]),
-}
-impl ::std::convert::From<[::std::string::String; 2usize]> for SingleTypeOneOfArrayBranch {
-    fn from(value: [::std::string::String; 2usize]) -> Self {
-        Self::Array(value)
-    }
-}
 #[doc = "`TypeArrayAllOfRefinement`"]
 #[doc = r""]
 #[doc = r" <details><summary>JSON schema</summary>"]
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"allOf is folded pairwise into the parent rather than producing branches. The type union must survive and the array-only constraints should apply when the Array variant is selected.\","]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"items\": {"]
@@ -109,7 +65,6 @@ impl ::std::convert::From<::std::vec::Vec<::std::string::String>> for TypeArrayA
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"Same shape as TypeArrayOneOfItems but using anyOf. anyOf travels through try_merge_with_each_subschema on a sibling path from oneOf; it should fold the type union the same way.\","]
 #[doc = "  \"anyOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"items\": {"]
@@ -154,7 +109,6 @@ impl ::std::convert::From<TypeArrayAnyOfItemsVariant1> for TypeArrayAnyOfItems {
 #[doc = "{"]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
-#[doc = "      \"$comment\": \"Same shape as TypeArrayOneOfItems but using anyOf. anyOf travels through try_merge_with_each_subschema on a sibling path from oneOf; it should fold the type union the same way.\","]
 #[doc = "      \"type\": ["]
 #[doc = "        \"string\","]
 #[doc = "        \"number\","]
@@ -202,7 +156,6 @@ impl ::std::convert::From<::std::vec::Vec<::std::string::String>> for TypeArrayA
 #[doc = "{"]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
-#[doc = "      \"$comment\": \"Same shape as TypeArrayOneOfItems but using anyOf. anyOf travels through try_merge_with_each_subschema on a sibling path from oneOf; it should fold the type union the same way.\","]
 #[doc = "      \"type\": ["]
 #[doc = "        \"string\","]
 #[doc = "        \"number\","]
@@ -248,7 +201,7 @@ impl ::std::convert::From<::std::vec::Vec<f64>> for TypeArrayAnyOfItemsVariant1
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"Case 9: every branch conflicts with the outer type union, so `try_merge_with_each_subschema` returns empty and the schema resolves to never. Must emit an empty enum cleanly rather than panic.\","]
+#[doc = "  \"$comment\": \"Every branch conflicts with the outer type union; must resolve cleanly rather than panic.\","]
 #[doc = "  \"oneOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"items\": {"]
@@ -292,7 +245,6 @@ pub enum TypeArrayFullyUnsatisfiableOneOf {}
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"not: object is redundant when the outer type union excludes object, but merging must not drop the type union when the not branch is applied.\","]
 #[doc = "  \"not\": {"]
 #[doc = "    \"type\": \"object\""]
 #[doc = "  },"]
@@ -327,7 +279,7 @@ impl ::std::convert::From<::std::vec::Vec<::serde_json::Value>> for TypeArrayNot
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"Case 10: oneOf and allOf on the same object, both alongside a multi-type `type` array. Exercises the full merge path (allOf folded first, then oneOf fanned out) with the Vec instance_type flowing through the merge arm.\","]
+#[doc = "  \"$comment\": \"oneOf and allOf on the same object, plus a multi-type `type` array.\","]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"minLength\": 1"]
@@ -604,7 +556,7 @@ impl<'de> ::serde::Deserialize<'de> for TypeArrayOneOfAndAllOfVariant1String {
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"Case 7: each oneOf branch pins `type: array`, so the intersection with the outer type union must prune the non-array primitives. Only array variants should be emitted.\","]
+#[doc = "  \"$comment\": \"Each oneOf branch pins `type: array`; the non-array variants from the outer union should be pruned.\","]
 #[doc = "  \"oneOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"items\": {"]
@@ -650,7 +602,6 @@ impl ::std::convert::From<::std::vec::Vec<f64>> for TypeArrayOneOfExplicitArrayB
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"Canonical issue #954 shape. Each oneOf branch only constrains `items`, so the type union must be folded into every branch rather than dropped.\","]
 #[doc = "  \"oneOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"items\": {"]
@@ -707,7 +658,6 @@ impl ::std::convert::From<TypeArrayOneOfItemsVariant2> for TypeArrayOneOfItems {
 #[doc = "{"]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
-#[doc = "      \"$comment\": \"Canonical issue #954 shape. Each oneOf branch only constrains `items`, so the type union must be folded into every branch rather than dropped.\","]
 #[doc = "      \"type\": ["]
 #[doc = "        \"string\","]
 #[doc = "        \"number\","]
@@ -769,7 +719,6 @@ impl ::std::convert::From<::std::vec::Vec<::std::string::String>> for TypeArrayO
 #[doc = "{"]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
-#[doc = "      \"$comment\": \"Canonical issue #954 shape. Each oneOf branch only constrains `items`, so the type union must be folded into every branch rather than dropped.\","]
 #[doc = "      \"type\": ["]
 #[doc = "        \"string\","]
 #[doc = "        \"number\","]
@@ -831,7 +780,6 @@ impl ::std::convert::From<::std::vec::Vec<f64>> for TypeArrayOneOfItemsVariant1
 #[doc = "{"]
 #[doc = "  \"allOf\": ["]
 #[doc = "    {"]
-#[doc = "      \"$comment\": \"Canonical issue #954 shape. Each oneOf branch only constrains `items`, so the type union must be folded into every branch rather than dropped.\","]
 #[doc = "      \"type\": ["]
 #[doc = "        \"string\","]
 #[doc = "        \"number\","]
@@ -891,7 +839,7 @@ impl ::std::convert::From<::std::vec::Vec<bool>> for TypeArrayOneOfItemsVariant2
 #[doc = r""]
 #[doc = r" ```json"]
 #[doc = "{"]
-#[doc = "  \"$comment\": \"Some oneOf branches conflict with the outer type union and should be dropped during merge; the surviving branch must carry the outer type union. The two eliminated branches use object/number which the outer `[string, array]` disallows.\","]
+#[doc = "  \"$comment\": \"Two oneOf branches conflict with the outer type union and should be dropped during merge.\","]
 #[doc = "  \"oneOf\": ["]
 #[doc = "    {"]
 #[doc = "      \"properties\": {"]