openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 17:38:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

detect object and array narrowing

Browse Source
This commit is contained in:
Adam Perry
2026-06-25 14:50:33 +00:00
parent 3944fc1292
commit 5974a03281
12 changed files with 559 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/schema-evolution/src/compare.rs
+4
View File
@@ -1,3 +1,5 @@
mod array;
mod object;
mod value;
use crate::ApiSchema;
@@ -152,6 +154,8 @@ fn compare_rules(
) -> 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);
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);
Ok(())
}
codex-rs/schema-evolution/src/compare/array.rs
+117
View File
@@ -0,0 +1,117 @@
use super::CompareCx;
use super::CompareNarrowing;
use crate::AdditionalItems;
use crate::ArraySchema;
use crate::Items;
use crate::SchemaId;
use crate::SchemaPath;
use anyhow::Result;
use serde_json::Value;
pub(super) fn compare_optional(
base: Option<&ArraySchema>,
current: Option<&ArraySchema>,
cx: &mut CompareCx<'_>,
path: &SchemaPath,
) -> Result<()> {
if base.is_none() && current.is_none() {
return Ok(());
}
let unconstrained = ArraySchema {
items: Items::Any,
additional_items: AdditionalItems::Any,
};
compare(
base.unwrap_or(&unconstrained),
current.unwrap_or(&unconstrained),
cx,
path,
)
}
fn compare(
base: &ArraySchema,
current: &ArraySchema,
cx: &mut CompareCx<'_>,
path: &SchemaPath,
) -> Result<()> {
let tuple_len = tuple_len(&base.items).max(tuple_len(&current.items));
for index in 0..tuple_len {
compare_element(
element_at(base, index),
element_at(current, index),
cx,
&path.tuple_item(index),
)?;
}
compare_element(tail(base), tail(current), cx, &path.items())
}
#[derive(Clone, Copy)]
enum Element {
Any,
Forbidden,
Schema(SchemaId),
}
fn tuple_len(items: &Items) -> usize {
match items {
Items::Tuple(items) => items.len(),
Items::Any | Items::Each(_) => 0,
}
}
fn element_at(array: &ArraySchema, index: usize) -> Element {
match &array.items {
Items::Any => Element::Any,
Items::Each(schema) => Element::Schema(*schema),
Items::Tuple(items) => items
.get(index)
.copied()
.map_or_else(|| additional(array.additional_items), Element::Schema),
}
}
fn tail(array: &ArraySchema) -> Element {
match array.items {
Items::Any => Element::Any,
Items::Each(schema) => Element::Schema(schema),
Items::Tuple(_) => additional(array.additional_items),
}
}
fn additional(items: AdditionalItems) -> Element {
match items {
AdditionalItems::Any => Element::Any,
AdditionalItems::Forbidden => Element::Forbidden,
AdditionalItems::Schema(schema) => Element::Schema(schema),
}
}
fn compare_element(
base: Element,
current: Element,
cx: &mut CompareCx<'_>,
path: &SchemaPath,
) -> Result<()> {
match (base, current) {
(Element::Forbidden, _) | (_, Element::Any) => Ok(()),
(Element::Schema(base), Element::Schema(current)) => base.compare(&current, cx, path),
(Element::Any, Element::Forbidden) => {
cx.constraint_changed(path, Value::Bool(true), Value::Bool(false));
Ok(())
}
(Element::Any, Element::Schema(current)) => {
cx.constraint_changed(path, Value::Bool(true), cx.current.snapshot(current)?);
Ok(())
}
(Element::Schema(base), Element::Forbidden) => {
cx.constraint_changed(path, cx.base.snapshot(base)?, Value::Bool(false));
Ok(())
}
}
}
#[cfg(test)]
#[path = "array_tests.rs"]
mod tests;
codex-rs/schema-evolution/src/compare/array_tests.rs
+64
View File
@@ -0,0 +1,64 @@
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 walks_homogeneous_and_tuple_items() -> anyhow::Result<()> {
let base = request_schema(json!({
"items": [{ "type": "string" }, { "type": "number" }],
"type": "array"
}));
let current = request_schema(json!({
"items": [{ "type": "string" }, { "type": "integer" }],
"type": "array"
}));
assert_eq!(
compare(&base, &current)?,
vec![breakage(
ViolationKind::TypeNarrowed,
"params[1]",
json!("number"),
json!("integer"),
)]
);
let base = request_schema(json!({ "items": { "type": "number" }, "type": "array" }));
let current = request_schema(json!({ "items": { "type": "integer" }, "type": "array" }));
assert_eq!(
compare(&base, &current)?,
vec![breakage(
ViolationKind::TypeNarrowed,
"params[]",
json!("number"),
json!("integer"),
)]
);
Ok(())
}
#[test]
fn detects_shortening_a_closed_tuple() -> anyhow::Result<()> {
let base = request_schema(json!({
"additionalItems": false,
"items": [{ "type": "string" }, { "type": "number" }],
"type": "array"
}));
let current = request_schema(json!({
"additionalItems": false,
"items": [{ "type": "string" }],
"type": "array"
}));
assert_eq!(
compare(&base, &current)?,
vec![breakage(
ViolationKind::ConstraintChanged,
"params[1]",
json!({ "type": "number" }),
json!(false),
)]
);
Ok(())
}
codex-rs/schema-evolution/src/compare/object.rs
+95
View File
@@ -0,0 +1,95 @@
use super::CompareCx;
use super::CompareNarrowing;
use crate::AdditionalProperties;
use crate::AdditionalPropertiesValue;
use crate::ObjectSchema;
use crate::SchemaPath;
use crate::Violation;
use anyhow::Result;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
pub(super) fn compare_optional(
base: Option<&ObjectSchema>,
current: Option<&ObjectSchema>,
cx: &mut CompareCx<'_>,
path: &SchemaPath,
) -> Result<()> {
if base.is_none() && current.is_none() {
return Ok(());
}
let unconstrained = ObjectSchema {
properties: BTreeMap::new(),
required: BTreeSet::new(),
additional_properties: AdditionalProperties::Any,
};
base.unwrap_or(&unconstrained)
.compare(current.unwrap_or(&unconstrained), cx, path)
}
impl CompareNarrowing for ObjectSchema {
fn compare(&self, current: &Self, cx: &mut CompareCx<'_>, path: &SchemaPath) -> Result<()> {
for (name, property) in &self.properties {
let property_path = path.property(name);
if let Some(current) = current.properties.get(name) {
property
.schema
.compare(&current.schema, cx, &property_path)?;
} else {
cx.violations.push(Violation::PropertyRemoved {
at: cx.location(&property_path),
});
}
}
for name in current.required.difference(&self.required) {
cx.violations.push(Violation::RequiredPropertyAdded {
at: cx.location(&path.property(name)),
});
}
compare_additional_properties(
self.additional_properties,
current.additional_properties,
cx,
&path.additional_properties(),
)
}
}
fn compare_additional_properties(
base: AdditionalProperties,
current: AdditionalProperties,
cx: &mut CompareCx<'_>,
path: &SchemaPath,
) -> Result<()> {
match (base, current) {
(AdditionalProperties::Forbidden, _) | (_, AdditionalProperties::Any) => Ok(()),
(AdditionalProperties::Schema(base), AdditionalProperties::Schema(current)) => {
base.compare(&current, cx, path)
}
(base, current) => {
let before = snapshot(base, cx.base)?;
let after = snapshot(current, cx.current)?;
cx.violations.push(Violation::AdditionalPropertiesNarrowed {
at: cx.location(path),
before,
after,
});
Ok(())
}
}
}
fn snapshot(
value: AdditionalProperties,
schema: &crate::ApiSchema,
) -> Result<AdditionalPropertiesValue> {
Ok(match value {
AdditionalProperties::Any => AdditionalPropertiesValue::Any,
AdditionalProperties::Forbidden => AdditionalPropertiesValue::Forbidden,
AdditionalProperties::Schema(id) => AdditionalPropertiesValue::Schema(schema.snapshot(id)?),
})
}
#[cfg(test)]
#[path = "object_tests.rs"]
mod tests;
codex-rs/schema-evolution/src/compare/object_tests.rs
+102
View File
@@ -0,0 +1,102 @@
use super::*;
use crate::ViolationKind;
use crate::test_support::breakage;
use crate::test_support::compare;
use crate::test_support::request_schema;
use crate::test_support::sorted;
use pretty_assertions::assert_eq;
use serde_json::json;
#[test]
fn detects_property_required_and_additional_property_narrowing() -> Result<()> {
let base = request_schema(json!({
"properties": {
"removed": { "type": "string" },
"requiredLater": { "type": "string" }
},
"type": "object"
}));
let current = request_schema(json!({
"additionalProperties": false,
"properties": { "requiredLater": { "type": "string" } },
"required": ["requiredLater"],
"type": "object"
}));
assert_eq!(
compare(&base, &current)?,
sorted(vec![
breakage(
ViolationKind::AdditionalPropertiesNarrowed,
"params.*",
json!(true),
json!(false),
),
breakage(
ViolationKind::PropertyRemoved,
"params.removed",
json!(true),
json!(false),
),
breakage(
ViolationKind::RequiredPropertyAdded,
"params.requiredLater",
json!(false),
json!(true),
),
])
);
Ok(())
}
#[test]
fn allows_object_widening() -> Result<()> {
let base = request_schema(json!({
"additionalProperties": false,
"properties": { "value": { "type": "string" } },
"required": ["value"],
"type": "object"
}));
let current = request_schema(json!({
"properties": {
"newOptional": { "type": "boolean" },
"value": { "type": "string" }
},
"type": "object"
}));
assert_eq!(compare(&base, &current)?, vec![]);
Ok(())
}
#[test]
fn reports_each_path_that_uses_a_narrowed_shared_definition() -> Result<()> {
let mut base = request_schema(json!({
"properties": {
"first": { "$ref": "#/definitions/Shared" },
"second": { "$ref": "#/definitions/Shared" }
},
"type": "object"
}));
base["definitions"] = json!({ "Shared": { "type": "number" } });
let mut current = base.clone();
current["definitions"]["Shared"]["type"] = json!("integer");
assert_eq!(
compare(&base, &current)?,
sorted(vec![
breakage(
ViolationKind::TypeNarrowed,
"params.first",
json!("number"),
json!("integer"),
),
breakage(
ViolationKind::TypeNarrowed,
"params.second",
json!("number"),
json!("integer"),
),
])
);
Ok(())
}
codex-rs/schema-evolution/src/compare/value_tests.rs
+47 -46
View File
@@ -2,58 +2,59 @@ use crate::ViolationKind;
use crate::test_support::breakage;
use crate::test_support::compare;
use crate::test_support::request_schema;
use crate::test_support::sorted;
use pretty_assertions::assert_eq;
use serde_json::json;
#[test]
fn detects_enum_type_and_bound_narrowing() -> anyhow::Result<()> {
let base = request_schema(json!({
"properties": {
"choice": { "enum": ["a", "b"], "type": "string" },
"length": { "minLength": 1, "type": "string" },
"numeric": { "type": "number" },
"value": { "type": ["string", "null"] }
},
"type": "object"
}));
let current = request_schema(json!({
"properties": {
"choice": { "enum": ["a"], "type": "string" },
"length": { "minLength": 2, "type": "string" },
"numeric": { "type": "integer" },
"value": { "type": "string" }
},
"type": "object"
}));
assert_eq!(
compare(
&request_schema(json!({ "enum": ["a", "b"], "type": "string" })),
&request_schema(json!({ "enum": ["a"], "type": "string" })),
)?,
vec![breakage(
ViolationKind::EnumNarrowed,
"params",
json!(["a", "b"]),
json!(["a"]),
)]
);
assert_eq!(
compare(
&request_schema(json!({ "type": "number" })),
&request_schema(json!({ "type": "integer" })),
)?,
vec![breakage(
ViolationKind::TypeNarrowed,
"params",
json!("number"),
json!("integer"),
)]
);
assert_eq!(
compare(
&request_schema(json!({ "type": ["string", "null"] })),
&request_schema(json!({ "type": "string" })),
)?,
vec![breakage(
ViolationKind::TypeNarrowed,
"params",
json!(["null", "string"]),
json!("string"),
)]
);
assert_eq!(
compare(
&request_schema(json!({ "minLength": 1, "type": "string" })),
&request_schema(json!({ "minLength": 2, "type": "string" })),
)?,
vec![breakage(
ViolationKind::ConstraintChanged,
"params",
json!({ "minLength": 1 }),
json!({ "minLength": 2 }),
)]
compare(&base, &current)?,
sorted(vec![
breakage(
ViolationKind::EnumNarrowed,
"params.choice",
json!(["a", "b"]),
json!(["a"]),
),
breakage(
ViolationKind::ConstraintChanged,
"params.length",
json!({ "minLength": 1 }),
json!({ "minLength": 2 }),
),
breakage(
ViolationKind::TypeNarrowed,
"params.numeric",
json!("number"),
json!("integer"),
),
breakage(
ViolationKind::TypeNarrowed,
"params.value",
json!(["null", "string"]),
json!("string"),
),
])
);
Ok(())
}
codex-rs/schema-evolution/src/compare_tests.rs
+27 -11
View File
@@ -39,22 +39,38 @@ fn compares_typed_arguments_with_the_full_request_schema() -> Result<()> {
)]
);
Ok(())
}
#[test]
fn retains_request_level_constraints_in_the_typed_envelope() -> Result<()> {
let base = request_schema(json!({ "type": "null" }));
let mut current = request_schema(json!({ "type": "null" }));
current["oneOf"][0]["minProperties"] = json!(3);
current["oneOf"][0]["properties"]["id"] = json!({ "type": "string" });
let mut expected = breakage(
ViolationKind::ConstraintChanged,
"request",
json!({}),
json!({ "minProperties": 3 }),
ViolationKind::TypeNarrowed,
"id",
json!(["integer", "string"]),
json!("string"),
);
expected.method = "*".to_string();
assert_eq!(compare(&base, &current)?, vec![expected]);
Ok(())
}
#[test]
fn retains_method_and_request_level_constraints_in_the_typed_envelope() -> Result<()> {
let mut base = request_schema(json!({ "type": "null" }));
base["oneOf"][0]["required"] = json!(["id", "params"]);
let mut current = request_schema(json!({ "type": "null" }));
current["oneOf"][0]["minProperties"] = json!(3);
let violations = compare(&base, &current)?;
assert_eq!(violations.len(), 2);
assert!(violations.iter().any(|violation| {
violation.kind == ViolationKind::RequiredPropertyAdded
&& violation.method == "*"
&& violation.path == "method"
}));
assert!(violations.iter().any(|violation| {
violation.kind == ViolationKind::ConstraintChanged
&& violation.method == "*"
&& violation.path == "request"
}));
Ok(())
}
codex-rs/schema-evolution/src/lib.rs
+5 -1
View File
@@ -7,10 +7,13 @@ mod violation;
mod test_support;
use compare::compare_api_schemas;
pub(crate) use model::AdditionalItems;
pub(crate) use model::AdditionalProperties;
pub use model::ApiSchema;
pub(crate) use model::Arguments;
pub(crate) use model::ArraySchema;
pub(crate) use model::ConstraintSet;
pub(crate) use model::Items;
pub(crate) use model::JsonType;
pub(crate) use model::Method;
pub(crate) use model::ObjectSchema;
@@ -21,6 +24,7 @@ pub(crate) use model::TypeSet;
pub(crate) use model::UnionKind;
pub(crate) use model::UnionSchema;
pub(crate) use model::ValueSet;
pub(crate) use violation::AdditionalPropertiesValue;
pub(crate) use violation::Location;
pub use violation::SchemaBreakage;
pub(crate) use violation::SchemaPath;
@@ -30,7 +34,7 @@ pub use violation::ViolationKind;
use anyhow::Result;
/// Finds values that the current request schema no longer accepts.
/// Finds request values accepted by `base` that are rejected by `current`.
pub fn find_request_narrowing(
base: &ApiSchema,
current: &ApiSchema,
codex-rs/schema-evolution/src/model.rs
+3
View File
@@ -5,7 +5,9 @@ mod object;
mod union;
mod value;
pub(crate) use array::AdditionalItems;
pub(crate) use array::ArraySchema;
pub(crate) use array::Items;
pub(crate) use constraints::ConstraintSet;
pub(crate) use constraints::annotation;
pub(crate) use constraints::normalized;
@@ -13,6 +15,7 @@ pub(crate) use constraints::normalized;
pub(crate) use method::Argument;
pub(crate) use method::Arguments;
pub(crate) use method::Method;
pub(crate) use object::AdditionalProperties;
pub(crate) use object::ObjectSchema;
pub(crate) use union::UnionKind;
pub(crate) use union::UnionSchema;
codex-rs/schema-evolution/src/test_support.rs
+5
View File
@@ -43,3 +43,8 @@ pub(crate) fn breakage(
after,
}
}
pub(crate) fn sorted(mut violations: Vec<SchemaBreakage>) -> Vec<SchemaBreakage> {
violations.sort_by_key(|violation| serde_json::to_string(violation).unwrap());
violations
}
codex-rs/schema-evolution/src/violation.rs
+59
View File
@@ -39,6 +39,11 @@ pub enum Violation {
before: Option<ValueSet>,
after: ValueSet,
},
AdditionalPropertiesNarrowed {
at: Location,
before: AdditionalPropertiesValue,
after: AdditionalPropertiesValue,
},
ConstraintChanged {
at: Location,
before: SchemaSnapshot,
@@ -64,6 +69,16 @@ pub struct SchemaPath(Vec<PathSegment>);
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
enum PathSegment {
Property(String),
Items,
TupleItem(usize),
AdditionalProperties,
}
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum AdditionalPropertiesValue {
Any,
Forbidden,
Schema(Value),
}
#[derive(Clone, Debug, Eq, PartialEq)]
@@ -95,6 +110,24 @@ impl SchemaPath {
path
}
pub(crate) fn items(&self) -> Self {
let mut path = self.clone();
path.0.push(PathSegment::Items);
path
}
pub(crate) fn tuple_item(&self, index: usize) -> Self {
let mut path = self.clone();
path.0.push(PathSegment::TupleItem(index));
path
}
pub(crate) fn additional_properties(&self) -> Self {
let mut path = self.clone();
path.0.push(PathSegment::AdditionalProperties);
path
}
pub(crate) fn starts_with_params(&self) -> bool {
matches!(self.0.first(), Some(PathSegment::Property(name)) if name == "params")
}
@@ -106,12 +139,26 @@ impl fmt::Display for SchemaPath {
match segment {
PathSegment::Property(name) if index == 0 => formatter.write_str(name)?,
PathSegment::Property(name) => write!(formatter, ".{name}")?,
PathSegment::Items => formatter.write_str("[]")?,
PathSegment::TupleItem(index) => write!(formatter, "[{index}]")?,
PathSegment::AdditionalProperties if index == 0 => formatter.write_str("*")?,
PathSegment::AdditionalProperties => formatter.write_str(".*")?,
}
}
Ok(())
}
}
impl AdditionalPropertiesValue {
fn to_json(&self) -> Value {
match self {
Self::Any => Value::Bool(true),
Self::Forbidden => Value::Bool(false),
Self::Schema(value) => value.clone(),
}
}
}
impl Violation {
pub fn breakage(&self) -> SchemaBreakage {
match self {
@@ -146,6 +193,12 @@ impl Violation {
before.as_ref().map_or(Value::Null, ValueSet::to_json),
after.to_json(),
),
Self::AdditionalPropertiesNarrowed { at, before, after } => at_location(
ViolationKind::AdditionalPropertiesNarrowed,
at,
before.to_json(),
after.to_json(),
),
Self::ConstraintChanged { at, before, after } => at_location(
ViolationKind::ConstraintChanged,
at,
@@ -162,6 +215,7 @@ impl Violation {
| Self::RequiredPropertyAdded { at }
| Self::TypeNarrowed { at, .. }
| Self::EnumNarrowed { at, .. }
| Self::AdditionalPropertiesNarrowed { at, .. }
| Self::ConstraintChanged { at, .. } => Some(at),
}
}
@@ -179,6 +233,7 @@ impl Violation {
| Self::RequiredPropertyAdded { at }
| Self::TypeNarrowed { at, .. }
| Self::EnumNarrowed { at, .. }
| Self::AdditionalPropertiesNarrowed { at, .. }
| Self::ConstraintChanged { at, .. } => Some(at),
}
}
@@ -199,3 +254,7 @@ fn at_location(kind: ViolationKind, at: &Location, before: Value, after: Value)
after,
}
}
#[cfg(test)]
#[path = "violation_tests.rs"]
mod tests;
codex-rs/schema-evolution/src/violation_tests.rs
+31
View File
@@ -0,0 +1,31 @@
use super::*;
use pretty_assertions::assert_eq;
use serde_json::json;
#[test]
fn breakage_projection_renders_typed_scope_paths_and_payloads() {
let at = Location::method(
"test/method",
SchemaPath::default()
.property("params")
.property("items")
.tuple_item(/*index*/ 1)
.additional_properties(),
);
let violation = Violation::ConstraintChanged {
at,
before: SchemaSnapshot(json!({ "minimum": 0 })),
after: SchemaSnapshot(json!({ "minimum": 1 })),
};
assert_eq!(
violation.breakage(),
SchemaBreakage {
kind: ViolationKind::ConstraintChanged,
method: "test/method".to_string(),
path: "params.items[1].*".to_string(),
before: json!({ "minimum": 0 }),
after: json!({ "minimum": 1 }),
}
);
}