From f2a62815308bd67cad64e6ffec98a70833f930ee Mon Sep 17 00:00:00 2001
From: Paul Masurel <paul.masurel@datadoghq.com>
Date: Wed, 9 Sep 2026 22:57:26 +0200
Subject: [PATCH] CR comment

---
 jitexpr/src/ast/infer_types.rs             |  8 ++++++++
 jitexpr/src/ast/literal.rs                 |  2 +-
 jitexpr/src/ast/mod.rs                     |  6 +++---
 jitexpr/src/ast/{serialize.rs => serde.rs} | 22 ++++++++++++----------
 4 files changed, 24 insertions(+), 14 deletions(-)
 rename jitexpr/src/ast/{serialize.rs => serde.rs} (97%)

diff --git a/jitexpr/src/ast/infer_types.rs b/jitexpr/src/ast/infer_types.rs
index 302771c99..74b8191c3 100644
--- a/jitexpr/src/ast/infer_types.rs
+++ b/jitexpr/src/ast/infer_types.rs
@@ -174,6 +174,11 @@ pub fn infer_types_with_target(
     Ok(inferred_type_res)
 }
 
+/// Infer the possible types of an UntypedExpr, meant to represent `target_inferred_type`.
+///
+/// As we call it recursively on the different nodes of the expression,
+/// this method should mutate the inferred_types (found in the inferred_type_res map) of each
+/// variable name encounterred, always restricting them.
 pub(crate) fn infer_types_aux<'a>(
     expr: &'a UntypedExpr,
     target_inferred_type: InferredTypeSet,
@@ -219,6 +224,7 @@ pub(crate) fn infer_type_with_variable_types(
     infer_types_aux(expr, target_inferred_type, &mut inferred_types)
 }
 
+/// Populate the inferred_types HashMap with the value types proved by the user.
 fn seed_variable_types<'a>(
     expr: &'a UntypedExpr,
     variable_types: &HashMap<&str, VarType>,
@@ -227,6 +233,8 @@ fn seed_variable_types<'a>(
     match expr {
         UntypedExpr::Literal(_) => {}
         UntypedExpr::Variable(variable_name) => {
+            // If the value is not provided by the user (for instance because we fed values from a
+            // columnar and no column with that column name exists), we treat it has being None.
             let inferred_type = variable_types
                 .get(variable_name.as_ref())
                 .copied()
diff --git a/jitexpr/src/ast/literal.rs b/jitexpr/src/ast/literal.rs
index 59751f2a0..a92175700 100644
--- a/jitexpr/src/ast/literal.rs
+++ b/jitexpr/src/ast/literal.rs
@@ -54,7 +54,7 @@ impl Literal {
         }
     }
 
-    // TODO let's remove it
+    #[cfg(test)]
     pub fn r#type(&self) -> VarType {
         match self {
             Literal::None => VarType::None,
diff --git a/jitexpr/src/ast/mod.rs b/jitexpr/src/ast/mod.rs
index 869c51c0a..fe5028a54 100644
--- a/jitexpr/src/ast/mod.rs
+++ b/jitexpr/src/ast/mod.rs
@@ -1,13 +1,13 @@
 mod infer_types;
 mod literal;
-mod serialize;
+mod serde;
 mod untyped_expr;
 
 pub use infer_types::{InferredTypeSet, TypeError, infer_types, infer_types_with_target};
 pub(crate) use infer_types::{infer_type_with_variable_types, infer_types_aux};
 pub use literal::Literal;
-pub(crate) use serialize::format_variable_name;
-pub use serialize::{DeserializeError, deserialize, serialize};
+pub(crate) use serde::format_variable_name;
+pub use serde::{DeserializeError, deserialize, serialize};
 pub use untyped_expr::UntypedExpr;
 
 pub use crate::functions::{Function, InvalidFnCall};
diff --git a/jitexpr/src/ast/serialize.rs b/jitexpr/src/ast/serde.rs
similarity index 97%
rename from jitexpr/src/ast/serialize.rs
rename to jitexpr/src/ast/serde.rs
index 1c71d0876..fdc573bd4 100644
--- a/jitexpr/src/ast/serialize.rs
+++ b/jitexpr/src/ast/serde.rs
@@ -1,4 +1,4 @@
-//! Serialization for [`UntypedExpr`] using a small Lisp-like syntax.
+//! De/Serialization for [`UntypedExpr`] using a small Lisp-like syntax.
 //!
 //! Calls are lists whose first item is a recognized uppercase function name.
 //! Elsewhere, atoms name variables unless they match a literal. For example:
@@ -8,18 +8,20 @@
 //! ```
 //!
 //! Numerical literals always carry a type suffix. Parsing rejects non-finite
-//! `f64` literals (NaN, infinities, and overflow). The other literals are
-//! `none`, `true`, `false`, and double-quoted strings. Backticks quote variable
-//! names containing whitespace or syntax characters, or matching literals:
+//! f64 literals (NaN, infinities, and overflow). The other literals are
+//! none, true, false, and double-quoted strings. Backticks quote variable
+//! names containing whitespace or syntax characters, or matching literals.
+//! In most case, backticks quote are unnecessary.
 //!
 //! ```text
-//! (ADD `1u64` 1u64)
+//! (ADD `text` 1u64)
+//! ```
+//! is the same as
+//! ```text
+//! (ADD text 1u64)
 //! ```
 //!
-//! Quoted variables use the same backslash escapes as strings, plus `` \` `` for
-//! a literal backtick. Serialization quotes names only when needed for an
-//! unambiguous round trip. Variable names are not restricted to ASCII or checked
-//! against a schema; field-name validation remains the caller's responsibility.
+//! Quoted variables use escaping to including quotation marks.
 
 use std::fmt;
 use std::sync::Arc;
@@ -121,7 +123,7 @@ fn format_literal(literal: &Literal, formatter: &mut fmt::Formatter) -> fmt::Res
     }
 }
 
-fn format_quoted(value: &str, quote: char, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+fn format_quoted(value: &str, quote: char, formatter: &mut fmt::Formatter) -> fmt::Result {
     write!(formatter, "{quote}")?;
     for character in value.chars() {
         match character {