addressed reviews

dbt_semantic_interfaces/implementations/metric.py

@@ -155,16 +155,6 @@ class PydanticConversionTypeParams(HashableBaseModel):
     window: Optional[PydanticMetricTimeWindow]
     constant_properties: Optional[List[PydanticConstantPropertyInput]]
 
-    @property
-    def base_measure_reference(self) -> MeasureReference:
-        """Return the measure reference associated with the base measure."""
-        return self.base_measure.measure_reference
-
-    @property
-    def conversion_measure_reference(self) -> MeasureReference:
-        """Return the measure reference associated with the conversion measure."""
-        return self.conversion_measure.measure_reference
-
 
 class PydanticMetricTypeParams(HashableBaseModel):
     """Type params add additional context to certain metric types (the context depends on the metric type)."""
@@ -180,12 +170,6 @@ class PydanticMetricTypeParams(HashableBaseModel):
 
     input_measures: List[PydanticMetricInputMeasure] = Field(default_factory=list)
 
-    @property
-    def conversion_type_params_or_error(self) -> PydanticConversionTypeParams:  # noqa: D
-        if self.conversion_type_params is None:
-            raise ValueError("Expected conversion_type_params to be not None.")
-        return self.conversion_type_params
-
 
 class PydanticMetric(HashableBaseModel, ModelWithMetadataParsing):
     """Describes a metric."""

dbt_semantic_interfaces/protocols/metric.py

@@ -118,9 +118,13 @@ def post_aggregation_reference(self) -> MetricReference:
 
 
 class ConstantPropertyInput(Protocol):
-    """Provides the constant property values for conversion metrics.
+    """Provides the constant property set for conversion metrics.
 
-    The specified property will be a reference of a dimension/entity.
+    Constant properties are additional elements linking a base event to a conversion event.
+    The specified properties will typically be a reference to a dimension or entity, and will be used
+    to join the base event to the final conversion event. Typical constant properties are things like
+    session keys (for services where conversions are measured within a user session), or secondary entities
+    (like a user/application pair for an app platform or a user/shop pair for a retail/online storefront platform).
     """
 
     @property
@@ -164,19 +168,7 @@ def calculation(self) -> ConversionCalculationType:
     @property
     @abstractmethod
     def window(self) -> Optional[MetricTimeWindow]:
-        """Maximum time range for finding successing conversion events."""
-        pass
-
-    @property
-    @abstractmethod
-    def base_measure_reference(self) -> MeasureReference:
-        """Return the measure reference associated with the base measure."""
-        pass
-
-    @property
-    @abstractmethod
-    def conversion_measure_reference(self) -> MeasureReference:
-        """Return the measure reference associated with the conversion measure."""
+        """Maximum time range for finding successive conversion events."""
         pass
 
     @property
@@ -235,11 +227,6 @@ def metrics(self) -> Optional[Sequence[MetricInput]]:  # noqa: D
     def conversion_type_params(self) -> Optional[ConversionTypeParams]:  # noqa: D
         pass
 
-    @property
-    @abstractmethod
-    def conversion_type_params_or_error(self) -> ConversionTypeParams:  # noqa: D
-        pass
-
 
 class Metric(Protocol):
     """Describes a metric."""

dbt_semantic_interfaces/transformations/add_input_metric_measures.py

@@ -43,8 +43,10 @@ def _get_measures_for_metric(
                         AddInputMetricMeasuresRule._get_measures_for_metric(semantic_manifest, input_metric.name)
                     )
             elif matched_metric.type is MetricType.CONVERSION:
-                measures.add(matched_metric.type_params.conversion_type_params_or_error.base_measure)
-                measures.add(matched_metric.type_params.conversion_type_params_or_error.conversion_measure)
+                conversion_type_params = matched_metric.type_params.conversion_type_params
+                assert conversion_type_params, "Conversion metric should have conversion_type_params."
+                measures.add(conversion_type_params.base_measure)
+                measures.add(conversion_type_params.conversion_measure)
             else:
                 assert_values_exhausted(matched_metric.type)
         else:

tests/test_implements_satisfy_protocols.py

@@ -20,6 +20,7 @@
 )
 from dbt_semantic_interfaces.implementations.metadata import PydanticMetadata
 from dbt_semantic_interfaces.implementations.metric import (
+    PydanticConversionTypeParams,
     PydanticMetric,
     PydanticMetricInput,
     PydanticMetricInputMeasure,
@@ -204,7 +205,15 @@ def test_metric_protocol_derived(metric: PydanticMetric) -> None:  # noqa: D
     builds(
         PydanticMetric,
         type=just(MetricType.CONVERSION),
-        type_params=builds(PydanticMetricTypeParams, metrics=lists(builds(PydanticMetricInput))),
+        type_params=builds(
+            PydanticMetricTypeParams,
+            conversion_type_params=builds(
+                PydanticConversionTypeParams,
+                base_measure=builds(PydanticMetricInputMeasure),
+                conversion_measure=builds(PydanticMetricInputMeasure),
+                entity=builds(str),
+            ),
+        ),
         expr=builds(str),
     )
 )