Clean up doc comments and add periods (#43)

## Problem

Cleans up a bit of the CPS PR.

## Summary of changes

This commit adds periods to all of the comments in the cir and engine. I
also added some more detail here and there.

This commit also unifies all of the types in the cir to make it easier
to interact with those types. Imo it makes more sense to have them all
in one place rather than to have to figure out the paths to each of them
every time you import.

Author: connortsui20

optd-core/src/bridge/from_cir.rs

@@ -1,10 +1,6 @@
-use crate::cir::{
-    goal::Goal,
-    group::GroupId,
-    operators::{Child, OperatorData},
-    plans::{PartialLogicalPlan, PartialPhysicalPlan},
-    properties::{LogicalProperties, PhysicalProperties, PropertiesData},
-};
+//! Converts optd's type representations (CIR) into DSL [`Value`]s (HIR).
+
+use crate::cir::*;
 use optd_dsl::analyzer::hir::{
     self, CoreData, Literal, LogicalOp, Materializable, Operator, PhysicalOp, Value,
 };
@@ -13,19 +9,15 @@ use CoreData::*;
 use Literal::*;
 use Materializable::*;
 
-//=============================================================================
-// Main conversion functions
-//=============================================================================
-
-/// Converts a PartialLogicalPlan into a HIR Value representation.
+/// Converts a [`PartialLogicalPlan`] into a [`Value`].
 pub(crate) fn partial_logical_to_value(plan: &PartialLogicalPlan) -> Value {
     match plan {
         PartialLogicalPlan::UnMaterialized(group_id) => {
-            // For unmaterialized logical operators, we create a Value with the group ID
+            // For unmaterialized logical operators, we create a `Value` with the group ID.
             Value(Logical(LogicalOp(UnMaterialized(hir::GroupId(group_id.0)))))
         }
         PartialLogicalPlan::Materialized(node) => {
-            // For materialized logical operators, we create a Value with the operator data
+            // For materialized logical operators, we create a `Value` with the operator data.
             let operator = Operator {
                 tag: node.tag.clone(),
                 data: convert_operator_data_to_values(&node.data),
@@ -37,11 +29,11 @@ pub(crate) fn partial_logical_to_value(plan: &PartialLogicalPlan) -> Value {
     }
 }
 
-/// Converts a PartialPhysicalPlan into a HIR Value representation.
+/// Converts a [`PartialPhysicalPlan`] into a [`Value`].
 pub(crate) fn partial_physical_to_value(plan: &PartialPhysicalPlan) -> Value {
     match plan {
         PartialPhysicalPlan::UnMaterialized(goal) => {
-            // For unmaterialized physical operators, we create a Value with the goal
+            // For unmaterialized physical operators, we create a `Value` with the goal
             let hir_goal = cir_goal_to_hir(goal);
             Value(Physical(PhysicalOp(UnMaterialized(hir_goal))))
         }
@@ -58,7 +50,7 @@ pub(crate) fn partial_physical_to_value(plan: &PartialPhysicalPlan) -> Value {
     }
 }
 
-/// Converts LogicalProperties to a HIR Value representation.
+/// Converts [`LogicalProperties`] into a [`Value`].
 #[allow(dead_code)]
 pub(crate) fn logical_properties_to_value(properties: &LogicalProperties) -> Value {
     match &properties.0 {
@@ -67,19 +59,15 @@ pub(crate) fn logical_properties_to_value(properties: &LogicalProperties) -> Val
     }
 }
 
-/// Converts PhysicalProperties to a HIR Value representation.
+/// Converts [`PhysicalProperties`] into a [`Value`].
 pub(crate) fn physical_properties_to_value(properties: &PhysicalProperties) -> Value {
     match &properties.0 {
         Some(data) => properties_data_to_value(data),
         None => Value(Null),
     }
 }
 
-//=============================================================================
-// CIR to HIR conversion helpers
-//=============================================================================
-
-/// Converts a CIR Goal to a HIR Goal.
+/// Converts a CIR [`Goal`] to a HIR [`Goal`](hir::Goal).
 fn cir_goal_to_hir(goal: &Goal) -> hir::Goal {
     let group_id = cir_group_id_to_hir(&goal.0);
     let properties = physical_properties_to_value(&goal.1);
@@ -90,12 +78,12 @@ fn cir_goal_to_hir(goal: &Goal) -> hir::Goal {
     }
 }
 
-/// Converts a CIR GroupId to a HIR GroupId.
+/// Converts a CIR [`GroupId`] to a HIR [`GroupId`](hir::GroupId).
 fn cir_group_id_to_hir(group_id: &GroupId) -> hir::GroupId {
     hir::GroupId(group_id.0)
 }
 
-/// Generic function to convert a Vec of Children to Vec of Values.
+/// A generic function to convert a slice of children into a vector of [`Value`]s.
 fn convert_children_to_values<T, F>(children: &[Child<Arc<T>>], converter: F) -> Vec<Value>
 where
     F: Fn(&T) -> Value,
@@ -112,12 +100,12 @@ where
         .collect()
 }
 
-/// Converts a slice of OperatorData to a Vec of HIR Values.
+/// Converts a slice of [`OperatorData`] into a vector of [`Value`]s.
 fn convert_operator_data_to_values(data: &[OperatorData]) -> Vec<Value> {
     data.iter().map(operator_data_to_value).collect()
 }
 
-/// Converts an OperatorData to a HIR Value representation.
+/// Converts an [`OperatorData`] into a [`Value`].
 fn operator_data_to_value(data: &OperatorData) -> Value {
     match data {
         OperatorData::Int64(i) => Value(Literal(Int64(*i))),
@@ -132,7 +120,7 @@ fn operator_data_to_value(data: &OperatorData) -> Value {
     }
 }
 
-/// Converts a PropertiesData to a HIR Value representation.
+/// Converts a [`PropertiesData`] into a [`Value`].
 fn properties_data_to_value(data: &PropertiesData) -> Value {
     match data {
         PropertiesData::Int64(i) => Value(Literal(Int64(*i))),

optd-core/src/bridge/into_cir.rs

@@ -1,24 +1,18 @@
-use crate::cir::{
-    goal::{Cost, Goal},
-    group::GroupId,
-    operators::{Child, Operator, OperatorData},
-    plans::{LogicalPlan, PartialLogicalPlan, PartialPhysicalPlan},
-    properties::{LogicalProperties, PhysicalProperties, PropertiesData},
-};
+//! Converts HIR [`Value`]s into optd's type representations (CIR).
+
+use crate::cir::*;
 use optd_dsl::analyzer::hir::{self, CoreData, Literal, Materializable, Value};
 use std::sync::Arc;
 use Child::*;
 use CoreData::*;
 use Literal::*;
 use Materializable::*;
 
-//=============================================================================
-// Main conversion functions
-//=============================================================================
-
-/// Converts a HIR Value into a PartialLogicalPlan representation.
+/// Converts a [`Value`] into a [`PartialLogicalPlan`].
 ///
-/// Transforms the DSL's HIR into the optimizer's IR for logical operators.
+/// # Panics
+///
+/// Panics if the [`Value`] is not a [`Logical`] variant.
 pub(crate) fn value_to_partial_logical(value: &Value) -> PartialLogicalPlan {
     match &value.0 {
         Logical(logical_op) => match &logical_op.0 {
@@ -35,9 +29,11 @@ pub(crate) fn value_to_partial_logical(value: &Value) -> PartialLogicalPlan {
     }
 }
 
-/// Converts a HIR Value into a PartialPhysicalPlan representation.
+/// Converts a [`Value`] into a [`PartialPhysicalPlan`].
+///
+/// # Panics
 ///
-/// Transforms the DSL's HIR into the CIR for physical operators.
+/// Panics if the [`Value`] is not a [`Physical`] variant.
 pub(crate) fn value_to_partial_physical(value: &Value) -> PartialPhysicalPlan {
     match &value.0 {
         Physical(physical_op) => match &physical_op.0 {
@@ -54,47 +50,57 @@ pub(crate) fn value_to_partial_physical(value: &Value) -> PartialPhysicalPlan {
     }
 }
 
-/// Converts a HIR Value into a CIR cost.
+/// Converts a [`Value`] into a CIR [`Cost`].
+///
+/// # Panics
+///
+/// Panics if the [`Value`] is not a [`Literal`] variant with a [`Float64`] value.
 pub(crate) fn value_to_cost(value: &Value) -> Cost {
     match &value.0 {
         Literal(Float64(f)) => Cost(*f),
         _ => panic!("Expected Float64 literal, found: {:?}", value.0),
     }
 }
 
-/// Convert HIR properties value to CIR LogicalProperties
+/// Converts an HIR properties [`Value`] into a CIR [`LogicalProperties`].
 pub(crate) fn value_to_logical_properties(properties_value: &Value) -> LogicalProperties {
     match &properties_value.0 {
         Null => LogicalProperties(None),
         _ => LogicalProperties(Some(value_to_properties_data(properties_value))),
     }
 }
 
-/// Converts a HIR GroupId to a CIR GroupId.
+/// Convert an HIR properties [`Value`] into a CIR [`PhysicalProperties`].
+fn value_to_physical_properties(properties_value: &Value) -> PhysicalProperties {
+    match &properties_value.0 {
+        Null => PhysicalProperties(None),
+        _ => PhysicalProperties(Some(value_to_properties_data(properties_value))),
+    }
+}
+
+/// Converts an HIR [`GroupId`](hir::GroupId) to a CIR [`GroupId`].
 ///
-/// This function provides a consistent way to convert group identifiers
-/// from the HIR representation to the optimizer's internal representation.
+/// This function provides a consistent way to convert group identifiers from the HIR into the
+/// optimizer's internal representation (CIR).
 pub(crate) fn hir_group_id_to_cir(hir_group_id: &hir::GroupId) -> GroupId {
     GroupId(hir_group_id.0)
 }
 
-/// Converts a HIR Goal to a CIR Goal.
-///
-/// Transforms the DSL's HIR Goal into the optimizer's IR Goal representation,
-/// handling both the group ID and physical properties components.
+/// Converts an HIR [`Goal`](hir::Goal) to a CIR [`Goal`].
 pub(crate) fn hir_goal_to_cir(hir_goal: &hir::Goal) -> Goal {
     let group_id = hir_group_id_to_cir(&hir_goal.group_id);
     let properties = value_to_physical_properties(&hir_goal.properties);
     Goal(group_id, properties)
 }
 
-//=============================================================================
-// HIR to CIR conversion helpers
-//=============================================================================
-
-/// Converts a HIR Value into a complete LogicalPlan (not a partial plan).
+/// Converts a [`Value`] into a fully materialized [`LogicalPlan`].
 ///
-/// Used when fully materializing a logical expression for use in properties.
+/// We use this function when materializing a logical expression for use in properties.
+///
+/// # Panics
+///
+/// Panics if the [`Value`] is not a [`Logical`] variant or if the [`Logical`] variant is not a
+/// [`Materialized`] variant.
 fn value_to_logical(value: &Value) -> LogicalPlan {
     match &value.0 {
         Logical(logical_op) => match &logical_op.0 {
@@ -111,15 +117,8 @@ fn value_to_logical(value: &Value) -> LogicalPlan {
     }
 }
 
-/// Convert HIR properties value to CIR PhysicalProperties
-fn value_to_physical_properties(properties_value: &Value) -> PhysicalProperties {
-    match &properties_value.0 {
-        Null => PhysicalProperties(None),
-        _ => PhysicalProperties(Some(value_to_properties_data(properties_value))),
-    }
-}
-
-/// Converts a Vec of Values to Vec of Children for any target type.
+/// A generic function to convert a slice of [`Value`]s into a vector of mapped results via the
+/// input `converter` function.
 fn convert_values_to_children<T, F>(values: &[Value], converter: F) -> Vec<Child<Arc<T>>>
 where
     F: Fn(&Value) -> T,
@@ -128,25 +127,32 @@ where
     values
         .iter()
         .map(|value| match &value.0 {
-            Array(elements) => {
-                VarLength(elements.iter().map(|el| Arc::new(converter(el))).collect())
-            }
+            Array(elements) => VarLength(
+                elements
+                    .iter()
+                    .map(|elem| Arc::new(converter(elem)))
+                    .collect(),
+            ),
             _ => Singleton(Arc::new(converter(value))),
         })
         .collect()
 }
 
-/// Converts a slice of HIR Values to a Vec of OperatorData.
+/// Converts a slice of [`Value`]s into a vector of [`OperatorData`].
 fn convert_values_to_operator_data(values: &[Value]) -> Vec<OperatorData> {
     values.iter().map(value_to_operator_data).collect()
 }
 
-/// Converts a slice of HIR Values to a Vec of PropertiesData.
+/// Converts a slice of [`Value`]s into a vector of [`PropertiesData`].
 fn convert_values_to_properties_data(values: &[Value]) -> Vec<PropertiesData> {
     values.iter().map(value_to_properties_data).collect()
 }
 
-/// Converts a HIR Value to an OperatorData representation.
+/// Converts a [`Value`] into an [`OperatorData`] representation.
+///
+/// # Panics
+///
+/// Panics if the [`Value`] cannot be converted to [`OperatorData`], such as a [`Unit`] literal.
 fn value_to_operator_data(value: &Value) -> OperatorData {
     match &value.0 {
         Literal(constant) => match constant {
@@ -164,7 +170,11 @@ fn value_to_operator_data(value: &Value) -> OperatorData {
     }
 }
 
-/// Converts a HIR Value to a PropertiesData representation.
+/// Converts a [`Value`] into a [`PropertiesData`] representation.
+///
+/// # Panics
+///
+/// Panics if the [`Value`] cannot be converted to [`PropertiesData`], such as a [`Unit`] literal.
 fn value_to_properties_data(value: &Value) -> PropertiesData {
     match &value.0 {
         Literal(constant) => match constant {

optd-core/src/bridge/mod.rs

@@ -1,13 +1,16 @@
-//! This module provides bridge functionality between the DSL's internal representation (HIR)
-//! and the query optimizer's intermediate representation (Optd-IR).
+//! This module provides bridge functionality between the DSL's internal representation (HIR) and
+//! the query optimizer's intermediate representation (Optd-IR).
 //!
 //! The bridge consists of two main components:
-//! - `into_optd`: Converts HIR Value objects into Optd's PartialPlan representations
-//! - `from_optd`: Converts Optd's PartialPlan representations into HIR Value objects
 //!
-//! These bidirectional conversions enable the DSL to interact with the query optimizer,
-//! allowing rule-based transformations to be applied to query plans while maintaining
-//! the ability to work with both representations.
+//! - [`from_cir`]: Converts optd's type representations (CIR) into DSL [`Value`]s (HIR).
+//! - [`into_cir`]: Converts HIR [`Value`]s into optd's type representations (CIR).
+//!
+//! These bidirectional conversions enable the DSL to interact with the query optimizer, allowing
+//! rule-based transformations to be applied to query plans while maintaining the ability to work
+//! with both representations.
+//!
+//! [`Value`]: optd_dsl::analyzer::hir::Value
 
 pub mod from_cir;
 pub mod into_cir;