feat(convert): simplify uptyped conversion and cover more scenarios (#826)

* feat(convert): simplify uptyped conversion and cover more scenarios

* test: add more cases

Author: georgeh0

python/cocoindex/convert.py

@@ -95,6 +95,7 @@ def make_engine_value_decoder(
     field_path: list[str],
     src_type: dict[str, Any],
     dst_type_info: AnalyzedTypeInfo,
+    for_key: bool = False,
 ) -> Callable[[Any], Any]:
     """
     Make a decoder from an engine value to a Python value.
@@ -123,6 +124,7 @@ def make_engine_value_decoder(
             field_path,
             src_type["fields"],
             dst_type_info,
+            for_key=for_key,
         )
 
     if src_type_kind in TABLE_TYPES:
@@ -131,18 +133,18 @@ def make_engine_value_decoder(
 
             if src_type_kind == "LTable":
                 if isinstance(dst_type_variant, AnalyzedAnyType):
-                    return _make_engine_ltable_to_list_dict_decoder(
-                        field_path, engine_fields_schema
-                    )
-                if not isinstance(dst_type_variant, AnalyzedListType):
+                    dst_elem_type = Any
+                elif isinstance(dst_type_variant, AnalyzedListType):
+                    dst_elem_type = dst_type_variant.elem_type
+                else:
                     raise ValueError(
                         f"Type mismatch for `{''.join(field_path)}`: "
                         f"declared `{dst_type_info.core_type}`, a list type expected"
                     )
                 row_decoder = make_engine_struct_decoder(
                     field_path,
                     engine_fields_schema,
-                    analyze_type_info(dst_type_variant.elem_type),
+                    analyze_type_info(dst_elem_type),
                 )
 
                 def decode(value: Any) -> Any | None:
@@ -152,10 +154,11 @@ def decode(value: Any) -> Any | None:
 
             elif src_type_kind == "KTable":
                 if isinstance(dst_type_variant, AnalyzedAnyType):
-                    return _make_engine_ktable_to_dict_dict_decoder(
-                        field_path, engine_fields_schema
-                    )
-                if not isinstance(dst_type_variant, AnalyzedDictType):
+                    key_type, value_type = Any, Any
+                elif isinstance(dst_type_variant, AnalyzedDictType):
+                    key_type = dst_type_variant.key_type
+                    value_type = dst_type_variant.value_type
+                else:
                     raise ValueError(
                         f"Type mismatch for `{''.join(field_path)}`: "
                         f"declared `{dst_type_info.core_type}`, a dict type expected"
@@ -166,13 +169,14 @@ def decode(value: Any) -> Any | None:
                 key_decoder = make_engine_value_decoder(
                     field_path,
                     key_field_schema["type"],
-                    analyze_type_info(dst_type_variant.key_type),
+                    analyze_type_info(key_type),
+                    for_key=True,
                 )
                 field_path.pop()
                 value_decoder = make_engine_struct_decoder(
                     field_path,
                     engine_fields_schema[1:],
-                    analyze_type_info(dst_type_variant.value_type),
+                    analyze_type_info(value_type),
                 )
 
                 def decode(value: Any) -> Any | None:
@@ -316,26 +320,26 @@ def make_engine_struct_decoder(
     field_path: list[str],
     src_fields: list[dict[str, Any]],
     dst_type_info: AnalyzedTypeInfo,
+    for_key: bool = False,
 ) -> Callable[[list[Any]], Any]:
     """Make a decoder from an engine field values to a Python value."""
 
     dst_type_variant = dst_type_info.variant
 
-    use_dict = False
     if isinstance(dst_type_variant, AnalyzedAnyType):
-        use_dict = True
+        if for_key:
+            return _make_engine_struct_to_tuple_decoder(field_path, src_fields)
+        else:
+            return _make_engine_struct_to_dict_decoder(field_path, src_fields, Any)
     elif isinstance(dst_type_variant, AnalyzedDictType):
         analyzed_key_type = analyze_type_info(dst_type_variant.key_type)
-        analyzed_value_type = analyze_type_info(dst_type_variant.value_type)
-        use_dict = (
+        if (
             isinstance(analyzed_key_type.variant, AnalyzedAnyType)
-            or (
-                isinstance(analyzed_key_type.variant, AnalyzedBasicType)
-                and analyzed_key_type.variant.kind == "Str"
+            or analyzed_key_type.core_type is str
+        ):
+            return _make_engine_struct_to_dict_decoder(
+                field_path, src_fields, dst_type_variant.value_type
             )
-        ) and isinstance(analyzed_value_type.variant, AnalyzedAnyType)
-    if use_dict:
-        return _make_engine_struct_to_dict_decoder(field_path, src_fields)
 
     if not isinstance(dst_type_variant, AnalyzedStructType):
         raise ValueError(
@@ -375,7 +379,7 @@ def make_closure_for_field(
         with ChildFieldPath(field_path, f".{name}"):
             if src_idx is not None:
                 field_decoder = make_engine_value_decoder(
-                    field_path, src_fields[src_idx]["type"], type_info
+                    field_path, src_fields[src_idx]["type"], type_info, for_key=for_key
                 )
                 return lambda values: field_decoder(values[src_idx])
 
@@ -409,17 +413,19 @@ def make_closure_for_field(
 def _make_engine_struct_to_dict_decoder(
     field_path: list[str],
     src_fields: list[dict[str, Any]],
+    value_type_annotation: Any,
 ) -> Callable[[list[Any] | None], dict[str, Any] | None]:
     """Make a decoder from engine field values to a Python dict."""
 
     field_decoders = []
-    for i, field_schema in enumerate(src_fields):
+    value_type_info = analyze_type_info(value_type_annotation)
+    for field_schema in src_fields:
         field_name = field_schema["name"]
         with ChildFieldPath(field_path, f".{field_name}"):
             field_decoder = make_engine_value_decoder(
                 field_path,
                 field_schema["type"],
-                analyze_type_info(Any),  # Use Any for recursive decoding
+                value_type_info,
             )
         field_decoders.append((field_name, field_decoder))
 
@@ -438,76 +444,37 @@ def decode_to_dict(values: list[Any] | None) -> dict[str, Any] | None:
     return decode_to_dict
 
 
-def _make_engine_ltable_to_list_dict_decoder(
+def _make_engine_struct_to_tuple_decoder(
     field_path: list[str],
     src_fields: list[dict[str, Any]],
-) -> Callable[[list[Any] | None], list[dict[str, Any]] | None]:
-    """Make a decoder from engine LTable values to a list of dicts."""
-
-    # Create a decoder for each row (struct) to dict
-    row_decoder = _make_engine_struct_to_dict_decoder(field_path, src_fields)
+) -> Callable[[list[Any] | None], tuple[Any, ...] | None]:
+    """Make a decoder from engine field values to a Python tuple."""
 
-    def decode_to_list_dict(values: list[Any] | None) -> list[dict[str, Any]] | None:
-        if values is None:
-            return None
-        result = []
-        for i, row_values in enumerate(values):
-            decoded_row = row_decoder(row_values)
-            if decoded_row is None:
-                raise ValueError(
-                    f"LTable row at index {i} decoded to None, which is not allowed."
+    field_decoders = []
+    value_type_info = analyze_type_info(Any)
+    for field_schema in src_fields:
+        field_name = field_schema["name"]
+        with ChildFieldPath(field_path, f".{field_name}"):
+            field_decoders.append(
+                make_engine_value_decoder(
+                    field_path,
+                    field_schema["type"],
+                    value_type_info,
                 )
-            result.append(decoded_row)
-        return result
-
-    return decode_to_list_dict
-
-
-def _make_engine_ktable_to_dict_dict_decoder(
-    field_path: list[str],
-    src_fields: list[dict[str, Any]],
-) -> Callable[[list[Any] | None], dict[Any, dict[str, Any]] | None]:
-    """Make a decoder from engine KTable values to a dict of dicts."""
-
-    if not src_fields:
-        raise ValueError("KTable must have at least one field for the key")
-
-    # First field is the key, remaining fields are the value
-    key_field_schema = src_fields[0]
-    value_fields_schema = src_fields[1:]
-
-    # Create decoders
-    with ChildFieldPath(field_path, f".{key_field_schema.get('name', KEY_FIELD_NAME)}"):
-        key_decoder = make_engine_value_decoder(
-            field_path, key_field_schema["type"], analyze_type_info(Any)
-        )
-
-    value_decoder = _make_engine_struct_to_dict_decoder(field_path, value_fields_schema)
+            )
 
-    def decode_to_dict_dict(
-        values: list[Any] | None,
-    ) -> dict[Any, dict[str, Any]] | None:
+    def decode_to_tuple(values: list[Any] | None) -> tuple[Any, ...] | None:
         if values is None:
             return None
-        result = {}
-        for row_values in values:
-            if not row_values:
-                raise ValueError("KTable row must have at least 1 value (the key)")
-            key = key_decoder(row_values[0])
-            if len(row_values) == 1:
-                value: dict[str, Any] = {}
-            else:
-                tmp = value_decoder(row_values[1:])
-                if tmp is None:
-                    value = {}
-                else:
-                    value = tmp
-            if isinstance(key, dict):
-                key = tuple(key.values())
-            result[key] = value
-        return result
+        if len(field_decoders) != len(values):
+            raise ValueError(
+                f"Field count mismatch: expected {len(field_decoders)}, got {len(values)}"
+            )
+        return tuple(
+            field_decoder(value) for value, field_decoder in zip(values, field_decoders)
+        )
 
-    return decode_to_dict_dict
+    return decode_to_tuple
 
 
 def dump_engine_object(v: Any) -> Any:

python/cocoindex/tests/test_convert.py

@@ -1160,6 +1160,37 @@ class MixedStruct:
     validate_full_roundtrip(instance, MixedStruct)
 
 
+def test_roundtrip_simple_struct_to_dict_binding() -> None:
+    """Test struct -> dict binding with Any annotation."""
+
+    @dataclass
+    class SimpleStruct:
+        first_name: str
+        last_name: str
+
+    instance = SimpleStruct("John", "Doe")
+    expected_dict = {"first_name": "John", "last_name": "Doe"}
+
+    # Test Any annotation
+    validate_full_roundtrip(
+        instance,
+        SimpleStruct,
+        (expected_dict, Any),
+        (expected_dict, dict),
+        (expected_dict, dict[Any, Any]),
+        (expected_dict, dict[str, Any]),
+        # For simple struct, all fields have the same type, so we can directly use the type as the dict value type.
+        (expected_dict, dict[Any, str]),
+        (expected_dict, dict[str, str]),
+    )
+
+    with pytest.raises(ValueError):
+        validate_full_roundtrip(instance, SimpleStruct, (expected_dict, dict[str, int]))
+
+    with pytest.raises(ValueError):
+        validate_full_roundtrip(instance, SimpleStruct, (expected_dict, dict[int, Any]))
+
+
 def test_roundtrip_struct_to_dict_binding() -> None:
     """Test struct -> dict binding with Any annotation."""
 
@@ -1173,7 +1204,20 @@ class SimpleStruct:
     expected_dict = {"name": "test", "value": 42, "price": 3.14}
 
     # Test Any annotation
-    validate_full_roundtrip(instance, SimpleStruct, (expected_dict, Any))
+    validate_full_roundtrip(
+        instance,
+        SimpleStruct,
+        (expected_dict, Any),
+        (expected_dict, dict),
+        (expected_dict, dict[Any, Any]),
+        (expected_dict, dict[str, Any]),
+    )
+
+    with pytest.raises(ValueError):
+        validate_full_roundtrip(instance, SimpleStruct, (expected_dict, dict[str, str]))
+
+    with pytest.raises(ValueError):
+        validate_full_roundtrip(instance, SimpleStruct, (expected_dict, dict[int, Any]))
 
 
 def test_roundtrip_struct_to_dict_explicit() -> None:
@@ -1289,7 +1333,13 @@ class User:
     ]
 
     # Test Any annotation
-    validate_full_roundtrip(users, list[User], (expected_list_dict, Any))
+    validate_full_roundtrip(
+        users,
+        list[User],
+        (expected_list_dict, Any),
+        (expected_list_dict, list[Any]),
+        (expected_list_dict, list[dict[str, Any]]),
+    )
 
 
 def test_roundtrip_ktable_to_dict_dict_binding() -> None:
@@ -1313,7 +1363,17 @@ class Product:
     }
 
     # Test Any annotation
-    validate_full_roundtrip(products, dict[str, Product], (expected_dict_dict, Any))
+    validate_full_roundtrip(
+        products,
+        dict[str, Product],
+        (expected_dict_dict, Any),
+        (expected_dict_dict, dict),
+        (expected_dict_dict, dict[Any, Any]),
+        (expected_dict_dict, dict[str, Any]),
+        (expected_dict_dict, dict[Any, dict[Any, Any]]),
+        (expected_dict_dict, dict[str, dict[Any, Any]]),
+        (expected_dict_dict, dict[str, dict[str, Any]]),
+    )
 
 
 def test_roundtrip_ktable_with_complex_key() -> None:
@@ -1339,7 +1399,28 @@ class Order:
     }
 
     # Test Any annotation
-    validate_full_roundtrip(orders, dict[OrderKey, Order], (expected_dict_dict, Any))
+    validate_full_roundtrip(
+        orders,
+        dict[OrderKey, Order],
+        (expected_dict_dict, Any),
+        (expected_dict_dict, dict),
+        (expected_dict_dict, dict[Any, Any]),
+        (expected_dict_dict, dict[Any, dict[str, Any]]),
+        (
+            {
+                ("shop1", 1): Order("Alice", 100.0),
+                ("shop2", 2): Order("Bob", 200.0),
+            },
+            dict[Any, Order],
+        ),
+        (
+            {
+                OrderKey("shop1", 1): {"customer": "Alice", "total": 100.0},
+                OrderKey("shop2", 2): {"customer": "Bob", "total": 200.0},
+            },
+            dict[OrderKey, Any],
+        ),
+    )
 
 
 def test_roundtrip_ltable_with_nested_structs() -> None: