Skip to content

Commit 113966f

Browse files
authored
Fix nested assert typechecking (#1966)
Closes #1962 <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Refactor nested type checking in BAML values to improve handling of nested assertions and add tests for validation. > > - **Behavior**: > - Refactor `distribute_type` in `mod.rs` to handle nested types using `distribute_type_with_meta`. > - Simplifies type distribution by using `BamlValueWithMeta::with_const_meta` and `map_meta_owned`. > - Adds test `test_distribute_optional_string_with_meta` to verify correct handling of optional strings with metadata. > - **Functions**: > - Modify `map_types` in `mod.rs` to handle unions with consistent key types and union value types. > - Update `coerce_arg` in `to_baml_arg.rs` to handle `FieldType::Arrow` and `FieldType::WithMetadata` more robustly. > - **Misc**: > - Minor formatting changes in `to_baml_arg.rs` for error messages. > - Update `BamlValueWithMeta` methods in `baml_value.rs` to support new type distribution logic. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup> for ea97654. You can [customize](https://app.ellipsis.dev/BoundaryML/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
1 parent ee15d0f commit 113966f

3 files changed

Lines changed: 107 additions & 180 deletions

File tree

engine/baml-lib/baml-core/src/ir/ir_helpers/mod.rs

Lines changed: 101 additions & 177 deletions
Original file line numberDiff line numberDiff line change
@@ -220,168 +220,11 @@ pub trait IRHelperExtended: IRSemanticStreamingHelper {
220220
value: BamlValue,
221221
field_type: FieldType,
222222
) -> anyhow::Result<BamlValueWithMeta<FieldType>> {
223-
match value {
224-
BamlValue::String(s) => {
225-
let literal_type = FieldType::Literal(LiteralValue::String(s.clone()));
226-
let primitive_type = FieldType::Primitive(TypeValue::String);
227-
228-
if self.is_subtype(&literal_type, &field_type)
229-
|| self.is_subtype(&primitive_type, &field_type)
230-
{
231-
return Ok(BamlValueWithMeta::String(s, field_type));
232-
}
233-
anyhow::bail!("Could not unify String with {:?}", field_type)
234-
}
235-
BamlValue::Int(i) => {
236-
let literal_type = FieldType::Literal(LiteralValue::Int(i));
237-
let primitive_type = FieldType::Primitive(TypeValue::Int);
238-
239-
if self.is_subtype(&literal_type, &field_type)
240-
|| self.is_subtype(&primitive_type, &field_type)
241-
{
242-
return Ok(BamlValueWithMeta::Int(i, field_type));
243-
}
244-
anyhow::bail!("Could not unify Int with {:?}", field_type)
245-
}
246-
247-
BamlValue::Float(f) => {
248-
if self.is_subtype(&FieldType::Primitive(TypeValue::Float), &field_type) {
249-
return Ok(BamlValueWithMeta::Float(f, field_type));
250-
}
251-
anyhow::bail!("Could not unify Float with {:?}", field_type)
252-
}
253-
254-
BamlValue::Bool(b) => {
255-
let literal_type = FieldType::Literal(LiteralValue::Bool(b));
256-
let primitive_type = FieldType::Primitive(TypeValue::Bool);
257-
258-
if self.is_subtype(&literal_type, &field_type)
259-
|| self.is_subtype(&primitive_type, &field_type)
260-
{
261-
Ok(BamlValueWithMeta::Bool(b, field_type))
262-
} else {
263-
anyhow::bail!("Could not unify Bool with {:?}", field_type)
264-
}
265-
}
266-
267-
BamlValue::Null
268-
if self.is_subtype(&FieldType::Primitive(TypeValue::Null), &field_type) =>
269-
{
270-
Ok(BamlValueWithMeta::Null(field_type))
271-
}
272-
BamlValue::Null => anyhow::bail!("Could not unify Null with {:?}", field_type),
273-
274-
BamlValue::Map(pairs) => {
275-
let item_types = pairs
276-
.iter()
277-
.filter_map(|(_, v)| infer_type(v))
278-
.dedup()
279-
.collect::<Vec<_>>();
280-
let maybe_item_type = match item_types.len() {
281-
0 => None,
282-
1 => Some(item_types[0].clone()),
283-
_ => Some(FieldType::Union(item_types)),
284-
};
285-
286-
match maybe_item_type {
287-
Some(item_type) => {
288-
let map_type = FieldType::Map(
289-
Box::new(match &field_type {
290-
FieldType::Map(key, _) => match key.as_ref() {
291-
FieldType::Enum(name) => FieldType::Enum(name.clone()),
292-
_ => FieldType::string(),
293-
},
294-
_ => FieldType::string(),
295-
}),
296-
Box::new(item_type.clone()),
297-
);
298-
299-
if !self.is_subtype(&map_type, &field_type) {
300-
anyhow::bail!("Could not unify {:?} with {:?}", map_type, field_type);
301-
}
302-
303-
let mapped_fields: BamlMap<String, BamlValueWithMeta<FieldType>> =
304-
pairs
305-
.into_iter()
306-
.map(|(key, val)| {
307-
let sub_value = self.distribute_type(val, item_type.clone())?;
308-
Ok((key, sub_value))
309-
})
310-
.collect::<anyhow::Result<BamlMap<String,BamlValueWithMeta<FieldType>>>>()?;
311-
Ok(BamlValueWithMeta::Map(mapped_fields, field_type))
312-
}
313-
None => Ok(BamlValueWithMeta::Map(BamlMap::new(), field_type)),
314-
}
315-
}
316-
317-
BamlValue::List(items) => {
318-
let item_types = items
319-
.iter()
320-
.filter_map(infer_type)
321-
.dedup()
322-
.collect::<Vec<_>>();
323-
let maybe_item_type = match item_types.len() {
324-
0 => None,
325-
1 => Some(item_types[0].clone()),
326-
_ => Some(FieldType::Union(item_types)),
327-
};
328-
match maybe_item_type.as_ref() {
329-
None => Ok(BamlValueWithMeta::List(vec![], field_type)),
330-
Some(item_type) => {
331-
let list_type = FieldType::List(Box::new(item_type.clone()));
332-
333-
if !self.is_subtype(&list_type, &field_type) {
334-
anyhow::bail!("Could not unify {:?} with {:?}", list_type, field_type);
335-
} else {
336-
let mapped_items: Vec<BamlValueWithMeta<FieldType>> = items
337-
.into_iter()
338-
.map(|i| self.distribute_type(i, item_type.clone()))
339-
.collect::<anyhow::Result<Vec<_>>>()?;
340-
Ok(BamlValueWithMeta::List(mapped_items, field_type))
341-
}
342-
}
343-
}
344-
}
345-
346-
BamlValue::Media(m)
347-
if self.is_subtype(
348-
&FieldType::Primitive(TypeValue::Media(m.media_type)),
349-
&field_type,
350-
) =>
351-
{
352-
Ok(BamlValueWithMeta::Media(m, field_type))
353-
}
354-
BamlValue::Media(_) => anyhow::bail!("Could not unify Media with {:?}", field_type),
355-
356-
BamlValue::Enum(name, val) => {
357-
if self.is_subtype(&FieldType::Enum(name.clone()), &field_type) {
358-
Ok(BamlValueWithMeta::Enum(name, val, field_type))
359-
} else {
360-
anyhow::bail!("Could not unify Enum {} with {:?}", name, field_type)
361-
}
362-
}
363-
364-
BamlValue::Class(name, fields) => {
365-
if !self.is_subtype(&FieldType::Class(name.clone()), &field_type) {
366-
anyhow::bail!("Could not unify Class {} with {:?}", name, field_type);
367-
} else {
368-
let class_fields = self.class_fields(&name)?;
369-
let mapped_fields = fields
370-
.into_iter()
371-
.map(|(k, v)| {
372-
let field_type = match class_fields.get(k.as_str()) {
373-
Some(ft) => ft.clone(),
374-
None => infer_type(&v).unwrap_or(UNIT_TYPE),
375-
};
376-
let mapped_field = self.distribute_type(v, field_type)?;
377-
Ok((k, mapped_field))
378-
})
379-
.collect::<anyhow::Result<BamlMap<String, BamlValueWithMeta<FieldType>>>>(
380-
)?;
381-
Ok(BamlValueWithMeta::Class(name, mapped_fields, field_type))
382-
}
383-
}
384-
}
223+
let value_with_empty_meta = BamlValueWithMeta::with_const_meta(&value, ());
224+
let res = self
225+
.distribute_type_with_meta(value_with_empty_meta, field_type)?
226+
.map_meta_owned(|(_, meta)| meta);
227+
Ok(res)
385228
}
386229

387230
/// For some `BamlValueWithMeta` with type `FieldType`, walk the structure of both the value
@@ -452,11 +295,9 @@ pub trait IRHelperExtended: IRSemanticStreamingHelper {
452295
let mapped_fields: BamlMap<String, BamlValueWithMeta<(T, FieldType)>> = pairs
453296
.into_iter()
454297
.map(|(key, val)| {
455-
let sub_value = item_type(self, &field_type, &val)
456-
.ok_or(anyhow::anyhow!(
457-
"Could not determine item_type of item in map"
458-
))
459-
.and_then(|item_type| self.distribute_type_with_meta(val, item_type))?;
298+
let sub_value =
299+
self.distribute_type_with_meta(val, annotation_value_type.clone())?;
300+
460301
Ok((key, sub_value))
461302
})
462303
.collect::<anyhow::Result<BamlMap<String, BamlValueWithMeta<(T, FieldType)>>>>(
@@ -1135,12 +976,12 @@ pub fn item_type<'ir, 'a, T: std::fmt::Debug>(
1135976
pub fn map_types<'ir, 'a>(
1136977
ir: &'ir (impl IRHelperExtended + ?Sized),
1137978
field_type: &'a FieldType,
1138-
) -> Option<(&'a FieldType, &'a FieldType)>
979+
) -> Option<(FieldType, FieldType)>
1139980
where
1140981
'ir: 'a,
1141982
{
1142983
match ir.distribute_metadata(field_type).0 {
1143-
FieldType::Map(key, value) => Some((key.as_ref(), value.as_ref())),
984+
FieldType::Map(key, value) => Some((*key.clone(), *value.clone())),
1144985
FieldType::RecursiveTypeAlias(alias_name) => ir
1145986
.recursive_alias_definition(alias_name)
1146987
.and_then(|alias_definition| map_types(ir, &alias_definition)),
@@ -1151,15 +992,33 @@ where
1151992
FieldType::Optional(base) => map_types(ir, base.as_ref()),
1152993
FieldType::Tuple(_) => None,
1153994
FieldType::Union(variants) => {
1154-
// When encountering a union, we return the key/value types of the
1155-
// first map we find inside the union.
1156-
// TODO: Give more thought to what `map_types` should return for
1157-
// unions, because the current logic is faulty for unions containing
1158-
// multiple maps.
1159-
let mut variant_map_types = variants
995+
let variant_map_types: Vec<(FieldType, FieldType)> = variants
1160996
.into_iter()
1161-
.filter_map(|variant| map_types(ir, variant));
1162-
variant_map_types.next()
997+
.filter_map(|variant| map_types(ir, variant))
998+
.collect();
999+
if variant_map_types.len() == 0 {
1000+
return None;
1001+
} else {
1002+
let first_key_type = variant_map_types[0].0.clone();
1003+
if !variant_map_types
1004+
.iter()
1005+
.all(|(key, _)| key == &first_key_type)
1006+
{
1007+
return None;
1008+
} else {
1009+
let value_types = variant_map_types
1010+
.into_iter()
1011+
.map(|(_, value_type)| value_type.clone())
1012+
.unique()
1013+
.collect::<Vec<_>>();
1014+
let value_type = match value_types.len() {
1015+
0 => None,
1016+
1 => Some(value_types[0].clone()),
1017+
_ => Some(FieldType::Union(value_types)),
1018+
}?;
1019+
return Some((first_key_type, value_type));
1020+
}
1021+
}
11631022
}
11641023
FieldType::Class(_) => None,
11651024
FieldType::Arrow(_) => None,
@@ -1592,6 +1451,71 @@ mod tests {
15921451
assert_eq!(base, &expected_base);
15931452
assert_eq!(constraints, expected_constraints);
15941453
}
1454+
1455+
#[test]
1456+
fn test_distribute_optional_string_with_meta() {
1457+
let ir = make_test_ir(r#""#).unwrap();
1458+
1459+
let res = ir
1460+
.distribute_type_with_meta(
1461+
BamlValueWithMeta::Null(()),
1462+
FieldType::WithMetadata {
1463+
base: Box::new(FieldType::Optional(Box::new(FieldType::Primitive(
1464+
TypeValue::String,
1465+
)))),
1466+
streaming_behavior: StreamingBehavior::default(),
1467+
constraints: vec![],
1468+
},
1469+
)
1470+
.expect("Distribution should succeed");
1471+
let res2 = ir
1472+
.distribute_type(
1473+
BamlValue::Null,
1474+
FieldType::WithMetadata {
1475+
base: Box::new(FieldType::Optional(Box::new(FieldType::Primitive(
1476+
TypeValue::String,
1477+
)))),
1478+
streaming_behavior: StreamingBehavior::default(),
1479+
constraints: vec![],
1480+
},
1481+
)
1482+
.expect("Distribution should succeed");
1483+
1484+
let res3 = ir
1485+
.distribute_type_with_meta(
1486+
BamlValueWithMeta::List(
1487+
vec![
1488+
BamlValueWithMeta::String("foo".to_string(), ()),
1489+
BamlValueWithMeta::String("bar".to_string(), ()),
1490+
],
1491+
(),
1492+
),
1493+
FieldType::WithMetadata {
1494+
base: Box::new(FieldType::List(Box::new(FieldType::Primitive(
1495+
TypeValue::String,
1496+
)))),
1497+
streaming_behavior: StreamingBehavior::default(),
1498+
constraints: vec![],
1499+
},
1500+
)
1501+
.expect("Distribution should succeed");
1502+
1503+
let res4 = ir
1504+
.distribute_type(
1505+
BamlValue::List(vec![
1506+
BamlValue::String("foo".to_string()),
1507+
BamlValue::String("bar".to_string()),
1508+
]),
1509+
FieldType::WithMetadata {
1510+
base: Box::new(FieldType::List(Box::new(FieldType::Primitive(
1511+
TypeValue::String,
1512+
)))),
1513+
streaming_behavior: StreamingBehavior::default(),
1514+
constraints: vec![],
1515+
},
1516+
)
1517+
.expect("Distribution should succeed");
1518+
}
15951519
}
15961520

15971521
// TODO: Copy pasted from baml-lib/baml-types/src/field_type/mod.rs and poorly

engine/baml-lib/baml-core/src/ir/ir_helpers/to_baml_arg.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,9 @@ impl ArgCoercer {
357357
}
358358
}
359359
(FieldType::Arrow(_), _) => {
360-
scope.push_error(format!("A json value may not be coerced into a function type"));
360+
scope.push_error(format!(
361+
"A json value may not be coerced into a function type"
362+
));
361363
Err(())
362364
}
363365
(FieldType::WithMetadata { .. }, _) => {

engine/baml-lib/baml-types/src/baml_value.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,8 @@ impl<T> BamlValueWithMeta<T> {
576576
),
577577
BamlValue::Media(m) => Media(m.clone(), meta),
578578
BamlValue::Enum(n, v) => Enum(n.clone(), v.clone(), meta),
579-
BamlValue::Class(_, items) => Map(
579+
BamlValue::Class(class_name, items) => BamlValueWithMeta::Class(
580+
class_name.clone(),
580581
items
581582
.iter()
582583
.map(|(k, v)| (k.clone(), Self::with_const_meta(v, meta.clone())))
@@ -660,7 +661,7 @@ impl<T> BamlValueWithMeta<T> {
660661
T: std::fmt::Debug,
661662
{
662663
let other_meta: U = other.meta().clone();
663-
let error_msg = String::new(); // format!("Could not unify {:?} with {:?}.", self, other);
664+
let error_msg = String::new();
664665
let ret = match (self, other) {
665666
(BamlValueWithMeta::Null(meta1), _) => {
666667
Result::<_, _>::Ok(BamlValueWithMeta::Null((meta1, other_meta)))

0 commit comments

Comments
 (0)