detect union schema narrowing

codex-rs/schema-evolution/src/compare.rs

@@ -1,5 +1,6 @@
 mod array;
 mod object;
+mod union;
 mod value;
 
 use crate::ApiSchema;
@@ -81,6 +82,34 @@ impl<'a> CompareCx<'a> {
             after: SchemaSnapshot(after),
         });
     }
+
+    pub(super) fn probe(
+        &self,
+        base: SchemaId,
+        current: SchemaId,
+        path: &SchemaPath,
+    ) -> Result<Vec<Violation>> {
+        let mut probe = Self::new(self.base, self.current, self.method);
+        probe.active.clone_from(&self.active);
+        base.compare(&current, &mut probe, path)?;
+        Ok(probe.violations)
+    }
+
+    pub(super) fn reverse_probe(
+        &self,
+        current: SchemaId,
+        base: SchemaId,
+        path: &SchemaPath,
+    ) -> Result<Vec<Violation>> {
+        let mut probe = Self::new(self.current, self.base, self.method);
+        probe.active = self
+            .active
+            .iter()
+            .map(|(base, current)| (*current, *base))
+            .collect();
+        current.compare(&base, &mut probe, path)?;
+        Ok(probe.violations)
+    }
 }
 
 impl CompareNarrowing for Method {
@@ -135,7 +164,7 @@ impl CompareNarrowing for SchemaId {
                 Ok(())
             }
             (SchemaNode::Rules(base), SchemaNode::Rules(current)) => {
-                compare_rules(base, current, cx, path)
+                compare_rules(base_id, base, current_id, current, cx, path)
             }
             (SchemaNode::Reference(_), _) | (_, SchemaNode::Reference(_)) => {
                 unreachable!("resolve returns concrete schema nodes")
@@ -147,13 +176,31 @@ impl CompareNarrowing for SchemaId {
 }
 
 fn compare_rules(
+    base_id: SchemaId,
     base: &SchemaRules,
+    current_id: SchemaId,
     current: &SchemaRules,
     cx: &mut CompareCx<'_>,
     path: &SchemaPath,
 ) -> Result<()> {
     value::compare_values(base.values.as_ref(), current.values.as_ref(), cx, path);
     value::compare_types(base.types.as_ref(), current.types.as_ref(), cx, path);
+    union::compare_optional(
+        base.any_of.as_ref(),
+        current.any_of.as_ref(),
+        base_id,
+        current_id,
+        cx,
+        path,
+    )?;
+    union::compare_optional(
+        base.one_of.as_ref(),
+        current.one_of.as_ref(),
+        base_id,
+        current_id,
+        cx,
+        path,
+    )?;
     object::compare_optional(base.object.as_ref(), current.object.as_ref(), cx, path)?;
     array::compare_optional(base.array.as_ref(), current.array.as_ref(), cx, path)?;
     value::compare_constraints(&base.constraints, &current.constraints, cx, path);

codex-rs/schema-evolution/src/compare/union.rs

@@ -0,0 +1,243 @@
+use super::CompareCx;
+use crate::ApiSchema;
+use crate::SchemaId;
+use crate::SchemaNode;
+use crate::SchemaPath;
+use crate::TypeSet;
+use crate::UnionKind;
+use crate::UnionSchema;
+use crate::ValueSet;
+use crate::VariantLabel;
+use crate::Violation;
+use anyhow::Result;
+use serde_json::Value;
+
+pub(super) fn compare_optional(
+    base: Option<&UnionSchema>,
+    current: Option<&UnionSchema>,
+    base_schema: SchemaId,
+    _current_schema: SchemaId,
+    cx: &mut CompareCx<'_>,
+    path: &SchemaPath,
+) -> Result<()> {
+    match (base, current) {
+        (None, None) | (Some(_), None) => Ok(()),
+        (None, Some(current)) => {
+            let mut covered = false;
+            for branch in &current.variants {
+                if cx.probe(base_schema, *branch, path)?.is_empty() {
+                    covered = true;
+                    break;
+                }
+            }
+            if !covered
+                || (current.kind == UnionKind::OneOf
+                    && !pairwise_disjoint(cx.current, &current.variants)?)
+            {
+                cx.constraint_changed(path, Value::Null, labels(cx.current, current)?);
+            }
+            Ok(())
+        }
+        (Some(base), Some(current)) => compare(base, current, cx, path),
+    }
+}
+
+fn compare(
+    base: &UnionSchema,
+    current: &UnionSchema,
+    cx: &mut CompareCx<'_>,
+    path: &SchemaPath,
+) -> Result<()> {
+    let mut all_covered = true;
+    for base_branch in &base.variants {
+        let key = branch_key(cx.base, *base_branch)?;
+        let mut matching_changes = None;
+        let mut covered = false;
+        for current_branch in &current.variants {
+            let changes = cx.probe(*base_branch, *current_branch, path)?;
+            if changes.is_empty() {
+                covered = true;
+            }
+            if matching_changes.is_none() && branch_key(cx.current, *current_branch)? == key {
+                matching_changes = Some(changes);
+            }
+        }
+        if !covered && let Some(changes) = matching_changes {
+            all_covered = false;
+            cx.violations.extend(changes);
+        } else if !covered {
+            all_covered = false;
+            cx.violations.push(Violation::UnionVariantRemoved {
+                at: cx.location(path),
+                variant: VariantLabel(key),
+            });
+        }
+    }
+    let all_equivalent = base.kind == UnionKind::OneOf
+        && all_covered
+        && base.variants.len() == current.variants.len()
+        && equivalent_bijection(base, current, cx, path)?;
+    if base.kind == UnionKind::OneOf
+        && all_covered
+        && !all_equivalent
+        && !pairwise_disjoint(cx.current, &current.variants)?
+    {
+        cx.constraint_changed(path, labels(cx.base, base)?, labels(cx.current, current)?);
+    }
+    Ok(())
+}
+
+fn equivalent_bijection(
+    base: &UnionSchema,
+    current: &UnionSchema,
+    cx: &CompareCx<'_>,
+    path: &SchemaPath,
+) -> Result<bool> {
+    let mut equivalent = vec![vec![false; current.variants.len()]; base.variants.len()];
+    for (base_index, base_branch) in base.variants.iter().enumerate() {
+        for (current_index, current_branch) in current.variants.iter().enumerate() {
+            equivalent[base_index][current_index] =
+                cx.probe(*base_branch, *current_branch, path)?.is_empty()
+                    && cx
+                        .reverse_probe(*current_branch, *base_branch, path)?
+                        .is_empty();
+        }
+    }
+    let mut assigned = vec![None; current.variants.len()];
+    for base_index in 0..base.variants.len() {
+        let mut seen = vec![false; current.variants.len()];
+        if !assign_equivalent(base_index, &equivalent, &mut assigned, &mut seen) {
+            return Ok(false);
+        }
+    }
+    Ok(true)
+}
+
+fn assign_equivalent(
+    base_index: usize,
+    equivalent: &[Vec<bool>],
+    assigned: &mut [Option<usize>],
+    seen: &mut [bool],
+) -> bool {
+    for current_index in 0..assigned.len() {
+        if !equivalent[base_index][current_index] || seen[current_index] {
+            continue;
+        }
+        seen[current_index] = true;
+        if assigned[current_index]
+            .is_none_or(|previous| assign_equivalent(previous, equivalent, assigned, seen))
+        {
+            assigned[current_index] = Some(base_index);
+            return true;
+        }
+    }
+    false
+}
+
+fn labels(schema: &ApiSchema, union: &UnionSchema) -> Result<Value> {
+    let mut labels = union
+        .variants
+        .iter()
+        .map(|variant| branch_key(schema, *variant).map(Value::String))
+        .collect::<Result<Vec<_>>>()?;
+    labels.sort_by_key(|label| serde_json::to_string(label).unwrap_or_default());
+    Ok(Value::Array(labels))
+}
+
+fn branch_key(schema: &ApiSchema, id: SchemaId) -> Result<String> {
+    let (_, node) = schema.resolve(id)?;
+    let SchemaNode::Rules(rules) = node else {
+        return Ok(format!(
+            "schema={}",
+            serde_json::to_string(&schema.snapshot(id)?)?
+        ));
+    };
+    if let Some(values) = &rules.values {
+        return Ok(format!("enum={}", serde_json::to_string(&values.values)?));
+    }
+    if let Some((name, values)) = discriminator(schema, id)? {
+        return Ok(format!("{name}={}", serde_json::to_string(&values.values)?));
+    }
+    if let Some(types) = &rules.types {
+        return Ok(format!("type={}", serde_json::to_string(&types.to_json())?));
+    }
+    Ok(format!(
+        "schema={}",
+        serde_json::to_string(&schema.snapshot(id)?)?
+    ))
+}
+
+fn pairwise_disjoint(schema: &ApiSchema, variants: &[SchemaId]) -> Result<bool> {
+    for (index, left) in variants.iter().enumerate() {
+        for right in &variants[index + 1..] {
+            if disjoint(schema, *left, *right)? {
+                continue;
+            }
+            return Ok(false);
+        }
+    }
+    Ok(true)
+}
+
+fn disjoint(schema: &ApiSchema, left: SchemaId, right: SchemaId) -> Result<bool> {
+    let left_types = types(schema, left)?;
+    let right_types = types(schema, right)?;
+    if matches!((left_types, right_types), (Some(left), Some(right)) if left.accepted_types().is_disjoint(&right.accepted_types()))
+    {
+        return Ok(true);
+    }
+    let left_values = values(schema, left)?;
+    let right_values = values(schema, right)?;
+    if matches!((left_values, right_values), (Some(left), Some(right)) if left.values.iter().all(|value| !right.values.contains(value)))
+    {
+        return Ok(true);
+    }
+    let left_discriminator = discriminator(schema, left)?;
+    let right_discriminator = discriminator(schema, right)?;
+    Ok(matches!(
+        (left_discriminator, right_discriminator),
+        (Some((left_name, left)), Some((right_name, right)))
+            if left_name == right_name
+                && left.values.iter().all(|value| !right.values.contains(value))
+    ))
+}
+
+fn types(schema: &ApiSchema, id: SchemaId) -> Result<Option<&TypeSet>> {
+    let (_, node) = schema.resolve(id)?;
+    Ok(match node {
+        SchemaNode::Rules(rules) => rules.types.as_ref(),
+        SchemaNode::Any | SchemaNode::Never => None,
+        SchemaNode::Reference(_) => unreachable!("resolve returns concrete schema nodes"),
+    })
+}
+
+fn values(schema: &ApiSchema, id: SchemaId) -> Result<Option<&ValueSet>> {
+    let (_, node) = schema.resolve(id)?;
+    Ok(match node {
+        SchemaNode::Rules(rules) => rules.values.as_ref(),
+        SchemaNode::Any | SchemaNode::Never => None,
+        SchemaNode::Reference(_) => unreachable!("resolve returns concrete schema nodes"),
+    })
+}
+
+fn discriminator(schema: &ApiSchema, id: SchemaId) -> Result<Option<(String, ValueSet)>> {
+    let (_, node) = schema.resolve(id)?;
+    let SchemaNode::Rules(rules) = node else {
+        return Ok(None);
+    };
+    let Some(object) = &rules.object else {
+        return Ok(None);
+    };
+    for (name, property) in &object.properties {
+        if object.required.contains(name)
+            && let Some(values) = values(schema, property.schema)?
+        {
+            return Ok(Some((name.clone(), values.clone())));
+        }
+    }
+    Ok(None)
+}
+
+#[cfg(test)]
+#[path = "union_tests.rs"]
+mod tests;

codex-rs/schema-evolution/src/compare/union_tests.rs

@@ -0,0 +1,88 @@
+use crate::ViolationKind;
+use crate::test_support::breakage;
+use crate::test_support::compare;
+use crate::test_support::request_schema;
+use pretty_assertions::assert_eq;
+use serde_json::json;
+
+#[test]
+fn detects_removed_union_variants_but_allows_widening() -> anyhow::Result<()> {
+    let base = request_schema(json!({
+        "anyOf": [{ "enum": ["a"] }, { "type": "null" }]
+    }));
+    let current = request_schema(json!({ "anyOf": [{ "enum": ["a", "b"] }] }));
+    assert_eq!(
+        compare(&base, &current)?,
+        vec![breakage(
+            ViolationKind::UnionVariantRemoved,
+            "params",
+            json!("type=\"null\""),
+            json!(null),
+        )]
+    );
+
+    let base = request_schema(json!({ "type": "string" }));
+    let current = request_schema(json!({
+        "anyOf": [{ "type": "string" }, { "type": "null" }]
+    }));
+    assert_eq!(compare(&base, &current)?, vec![]);
+    Ok(())
+}
+
+#[test]
+fn detects_new_overlap_in_one_of() -> anyhow::Result<()> {
+    let base = request_schema(json!({
+        "oneOf": [{ "enum": ["a"] }, { "enum": ["b"] }]
+    }));
+    let current = request_schema(json!({
+        "oneOf": [{ "enum": ["a", "b"] }, { "enum": ["b", "c"] }]
+    }));
+    let violations = compare(&base, &current)?;
+    assert_eq!(violations.len(), 1);
+    assert_eq!(violations[0].kind, ViolationKind::ConstraintChanged);
+    assert_eq!(violations[0].path, "params");
+    Ok(())
+}
+
+#[test]
+fn compares_recursive_union_refs_without_reentering_forever() -> anyhow::Result<()> {
+    let mut base = request_schema(json!({ "$ref": "#/definitions/Params" }));
+    base["definitions"] = json!({
+        "Params": {
+            "properties": {
+                "child": { "anyOf": [{ "$ref": "#/definitions/Params" }, { "type": "null" }] },
+                "value": { "type": "number" }
+            },
+            "type": "object"
+        }
+    });
+    let mut current = base.clone();
+    current["definitions"]["Params"]["properties"]["value"]["type"] = json!("integer");
+
+    assert_eq!(
+        compare(&base, &current)?,
+        vec![breakage(
+            ViolationKind::TypeNarrowed,
+            "params.value",
+            json!("number"),
+            json!("integer"),
+        )]
+    );
+    Ok(())
+}
+
+#[test]
+fn one_of_equivalence_preserves_duplicate_branch_multiplicity() -> anyhow::Result<()> {
+    let base = request_schema(json!({
+        "oneOf": [{ "enum": ["a"] }, { "enum": ["a"] }, { "enum": ["b"] }]
+    }));
+    let current = request_schema(json!({
+        "oneOf": [{ "enum": ["a"] }, { "enum": ["b"] }, { "enum": ["b"] }]
+    }));
+
+    let violations = compare(&base, &current)?;
+    assert_eq!(violations.len(), 1);
+    assert_eq!(violations[0].kind, ViolationKind::ConstraintChanged);
+    assert_eq!(violations[0].path, "params");
+    Ok(())
+}

codex-rs/schema-evolution/src/compare_tests.rs

@@ -53,6 +53,26 @@ fn compares_typed_arguments_with_the_full_request_schema() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn detects_request_level_union_narrowing_that_constrains_params() -> Result<()> {
+    let base = request_schema(json!({ "type": "number" }));
+    let mut current = base.clone();
+    current["oneOf"][0]["anyOf"] = json!([{
+        "properties": {
+            "id": { "type": ["string", "integer"] },
+            "method": { "enum": ["test/method"], "type": "string" },
+            "params": { "type": "integer" }
+        },
+        "type": "object"
+    }]);
+
+    let violations = compare(&base, &current)?;
+    assert!(violations.iter().any(|violation| {
+        violation.kind == ViolationKind::ConstraintChanged && violation.path == "request"
+    }));
+    Ok(())
+}
+
 #[test]
 fn retains_method_and_request_level_constraints_in_the_typed_envelope() -> Result<()> {
     let mut base = request_schema(json!({ "type": "null" }));

codex-rs/schema-evolution/src/lib.rs

@@ -29,6 +29,7 @@ pub(crate) use violation::Location;
 pub use violation::SchemaBreakage;
 pub(crate) use violation::SchemaPath;
 pub(crate) use violation::SchemaSnapshot;
+pub(crate) use violation::VariantLabel;
 pub(crate) use violation::Violation;
 pub use violation::ViolationKind;
 

codex-rs/schema-evolution/src/violation.rs

@@ -39,6 +39,10 @@ pub enum Violation {
         before: Option<ValueSet>,
         after: ValueSet,
     },
+    UnionVariantRemoved {
+        at: Location,
+        variant: VariantLabel,
+    },
     AdditionalPropertiesNarrowed {
         at: Location,
         before: AdditionalPropertiesValue,
@@ -74,6 +78,9 @@ enum PathSegment {
     AdditionalProperties,
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct VariantLabel(pub String);
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum AdditionalPropertiesValue {
     Any,
@@ -193,6 +200,12 @@ impl Violation {
                 before.as_ref().map_or(Value::Null, ValueSet::to_json),
                 after.to_json(),
             ),
+            Self::UnionVariantRemoved { at, variant } => at_location(
+                ViolationKind::UnionVariantRemoved,
+                at,
+                Value::String(variant.0.clone()),
+                Value::Null,
+            ),
             Self::AdditionalPropertiesNarrowed { at, before, after } => at_location(
                 ViolationKind::AdditionalPropertiesNarrowed,
                 at,
@@ -215,6 +228,7 @@ impl Violation {
             | Self::RequiredPropertyAdded { at }
             | Self::TypeNarrowed { at, .. }
             | Self::EnumNarrowed { at, .. }
+            | Self::UnionVariantRemoved { at, .. }
             | Self::AdditionalPropertiesNarrowed { at, .. }
             | Self::ConstraintChanged { at, .. } => Some(at),
         }
@@ -233,6 +247,7 @@ impl Violation {
             | Self::RequiredPropertyAdded { at }
             | Self::TypeNarrowed { at, .. }
             | Self::EnumNarrowed { at, .. }
+            | Self::UnionVariantRemoved { at, .. }
             | Self::AdditionalPropertiesNarrowed { at, .. }
             | Self::ConstraintChanged { at, .. } => Some(at),
         }