feat: Implement WRAP_ACTIONS with EXPR binding to RS (#285)

* feat: Implement WRAP_ACTIONS with EXPR binding to RS

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: Pass conformance test

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: add build_symbol_table binding for typed EXPR symbol tables

Add a build_symbol_table #[pyfunction] that constructs a typed
openjd-expr SymbolTable from a flat dotted-key value map plus an
optional per-key EXPR type-spec map. String values are coerced to
their declared type via ExprValue::from_str_coerce and dotted keys
are nested into subtables.

This moves the typed symbol-table construction next to the engine
(previously a string-coercion + nesting dance in the Python
_expr_support bridge), keeping value typing consistent with
evaluation and giving correct native-aggregate inference. The
OpenJD-type -> EXPR-type-spec mapping stays on the Python side and is
passed in as resolved spec strings.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: expr and wrap_actions pr comments

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: v0 parity for RFC 0007 typed params and RFC 0008 wrap-action rules

Close the remaining pure-Python v0 vs Rust gaps for the EXPR / WRAP_ACTIONS
extensions in the default model path:

- RFC 0007: v0 create_job can now instantiate jobs that declare BOOL,
  RANGE_EXPR, and LIST[*] parameters. Their values are carried natively
  (lists/bools) through preprocessing into the JobParameter, and the typed
  EXPR symbol table (build_symbol_table) coerces them, rather than the
  string-only handling used by the original scalar types. ParameterValueType,
  ParameterValue.value, and JobParameter.value are extended accordingly, and
  the parameter merge skips the legacy scalar-only constraint merging (and the
  EXPR-gated re-parse) for the new types.

- RFC 0008 single-wrap-layer: JobTemplate now rejects more than one
  wrap-defining environment reachable in any session stack (jobEnvironments
  plus one step's stepEnvironments), mirroring openjd-rs wrap_actions.rs.

- RFC 0008 wrapped-variable scoping: EnvironmentActions now rejects
  WrappedAction.* outside the wrap hooks, WrappedEnv.* outside the env
  enter/exit hooks, and WrappedStep.* outside the task-run hook.

Runtime action-wrapping (RFC 0008 execution) lives in
openjd-sessions-for-python and is tracked as a separate change.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: reuse Rust via bindings to simplify the EXPR model glue

Replace hand-maintained Python logic in the v0 model's EXPR glue with thin
calls into the Rust bindings, so the behaviour is single-sourced in the
openjd crates:

- job_parameter_type_expr_spec: maps an OpenJD job-parameter type name
  (case-insensitive) to its EXPR type spec by reusing the crate's
  JobParameterType::from_spec_str + expr_type(). Removes the hand-maintained
  _OPENJD_TYPE_TO_EXPR_TYPE table and the recursive LIST[...] unwrap from
  _expr_support.py; the OpenJD-type -> EXPR-type contract (incl. nested
  LIST[LIST[INT]]) now lives only in the openjd-model crate. RANGE_EXPR now
  maps to the engine's range_expr type.

- ParsedExpression.typecheck: type-checks an expression against typed
  (unresolved) symbol placeholders without extracting a result. Lets
  validate_typed_expression drop its brittle error-message sniffing
  ("unresolved" / "Cannot extract value") — a well-typed but runtime-dependent
  expression now simply type-checks Ok in Rust instead of surfacing a
  boundary error the Python had to recognise by string.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: scope-check timeout format strings and cover RANGE_EXPR typed validation

RFC 0008 wrapped-variable scoping: _action_referenced_namespaces now also
inspects an Action's timeout field, which is a FormatString under the
FEATURE_BUNDLE_1 extension. Previously only command/args were scanned, so a
WrappedAction.*/WrappedEnv.*/WrappedStep.* reference smuggled into a timeout
expression bypassed the per-hook scope rule. Non-FormatString timeouts (plain
ints) are skipped by the existing isinstance guard.

RFC 0007 RANGE_EXPR: add tests pinning the typed-validation behaviour now that
the OpenJD-type -> EXPR-type mapping resolves RANGE_EXPR to the engine's
range_expr type (previously None, which fell back to name-only validation).
Valid range_expr ops (subscript, len, list) type-check; genuine type errors
(arithmetic, invalid methods) are now caught at decode time. Covered at the
ExprNode seam and end-to-end through decode_job_template.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* feat: harden EXPR/WRAP_ACTIONS v0 model — spec string coercion, value constraints, lazy import

Address six review findings on the EXPR (RFC 0005-0007) / WRAP_ACTIONS (RFC
0008) support in the pure-Python (v0) model:

- EXPR results interpolated into format strings now use the engine's
  RFC 0005 string coercion (true/false, double-quoted list items, preserved
  Decimal trailing zeros; null -> empty string) instead of Python str() of the
  native value, which emitted Python reprs (True/None/['a', 'b']). Adds
  evaluate_to_str() to the node/expression seam; evaluate() still returns the
  native value.
- create_job/preprocess now enforce item/length/range constraints on
  user-supplied LIST[*] and RANGE_EXPR values (previously only the template
  default was validated at decode time).
- Merged EXPR defaults are re-validated against the surviving definition's
  constraints before model_copy (which skips validators), so a default carried
  over from another source can no longer bypass them.
- SymbolTable.expr_types is now a first-class field copied by __init__ and
  union(), instead of a dynamic attribute that derived/unioned tables dropped.
- The EXPR_EXTENSION gate constant moved to _parser so importing the parser no
  longer eagerly loads the Rust expr surface on the non-EXPR path.
- Collapsed the eight near-identical JobList*/RangeExpr definitions onto a
  shared base, and single-sourced the triplicated task-parameter range gate.

Adds 27 regression unit tests covering each fix.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

* test: cover create_job instantiation of WRAP_ACTIONS templates

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

---------

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

Author: leongdl

rust-bindings/src/expr/mod.rs

@@ -26,5 +26,5 @@ pub(crate) use path_mapping::PyPathMappingRule;
 pub(crate) use profile::{PyExprExtension, PyExprProfile, PyExprRevision, PyHostContext};
 pub(crate) use range_expr::{PyIntRange, PyRangeExpr};
 pub(crate) use symbol_table::{
-    _reconstruct_serialized_symtab, PySerializedSymbolTable, PySymbolTable,
+    _reconstruct_serialized_symtab, build_symbol_table, PySerializedSymbolTable, PySymbolTable,
 };

rust-bindings/src/expr/parsed_expression.rs

@@ -92,6 +92,37 @@ impl PyParsedExpression {
         Ok(PyExprValue { inner: value })
     }
 
+    /// Type-check the expression against a symbol table of (typically
+    /// unresolved) typed placeholders, without extracting a concrete value.
+    ///
+    /// Succeeds when the expression is well-typed for the given symbol types —
+    /// including when the result is an unresolved value that merely depends on
+    /// a runtime symbol — and raises only on a genuine type/evaluation error.
+    /// Unlike :meth:`evaluate`, it discards the result, so it never raises the
+    /// "cannot extract value from unresolved" boundary error; callers no longer
+    /// need to sniff the error message to tell a real type error from a
+    /// runtime-dependent one.
+    #[pyo3(signature = (*, values=None, profile=None))]
+    fn typecheck(
+        &self,
+        values: Option<&Bound<'_, pyo3::PyAny>>,
+        profile: Option<&PyExprProfile>,
+    ) -> PyResult<()> {
+        let symtab;
+        let symtab_refs: Vec<&SymbolTable> = if let Some(v) = values {
+            symtab = extract_symtab(v)?;
+            vec![&symtab]
+        } else {
+            vec![]
+        };
+        let lib = profile_for_call(profile);
+        self.inner
+            .with_library(&lib)
+            .evaluate(&symtab_refs)
+            .map_err(expr_err_to_py)?;
+        Ok(())
+    }
+
     /// Evaluate the expression and return an :class:`EvalResult` with the
     /// resulting value alongside the per-call resource-usage metrics
     /// (``peak_memory`` in bytes, ``operation_count``).

rust-bindings/src/expr/symbol_table.rs

@@ -2,13 +2,17 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use pyo3::prelude::*;
-use pyo3::types::PyDict;
+use pyo3::types::{PyDict, PyString};
 #[cfg(feature = "stub-gen")]
 use pyo3_stub_gen::derive::*;
 
+use openjd_expr::path_mapping::PathFormat;
 use openjd_expr::symbol_table::SymbolTable;
+use openjd_expr::types::ExprType;
+use openjd_expr::value::ExprValue;
 
 use crate::expr::expr_value::{py_to_expr_value, PyExprValue};
+use crate::expr::path_format::PyPathFormat;
 
 #[cfg_attr(feature = "stub-gen", gen_stub_pyclass(module = "openjd._openjd_rs"))]
 #[pyclass(module = "openjd.expr", name = "SymbolTable", from_py_object)]
@@ -324,3 +328,79 @@ pub(crate) fn _reconstruct_serialized_symtab(json: &str) -> PyResult<PySerialize
     })?;
     Ok(PySerializedSymbolTable { inner })
 }
+
+// ── Typed symbol-table builder ─────────────────────────────────────
+//
+// Bridges the pure-Python (v0) model's flat dotted-key symbol table into
+// a typed `SymbolTable` the engine can evaluate against. This replaces the
+// former Python `symtab_to_expr_values`/`_to_expr_value` coercion so the
+// string→typed-value coercion lives next to the engine (PR #285 review,
+// C3/C5). The OpenJD-type → EXPR-type-spec mapping stays in Python; this
+// function takes the resolved EXPR type spec strings (e.g. "int",
+// "list[int]", "path") so the engine owns only the coercion + nesting.
+
+/// Coerce a single Python value to an `ExprValue`, optionally toward a known
+/// target `ExprType`. String values are coerced via `from_str_coerce` (so a
+/// stored ``"10"`` of type INT becomes a real integer); other native values
+/// are built then coerced. With no target the value's native type is
+/// inferred. Mirrors the former Python ``_expr_support._to_expr_value``.
+fn coerce_symbol_value(
+    value: &Bound<'_, pyo3::PyAny>,
+    target: Option<&ExprType>,
+    pf: PathFormat,
+) -> PyResult<ExprValue> {
+    let Some(target) = target else {
+        // No confident type — let the engine infer from the native value.
+        return py_to_expr_value(value);
+    };
+    if let Ok(s) = value.cast::<PyString>() {
+        return ExprValue::from_str_coerce(&s.to_cow()?, target, pf)
+            .map_err(pyo3::exceptions::PyValueError::new_err);
+    }
+    py_to_expr_value(value)?
+        .coerce(target, pf)
+        .map_err(pyo3::exceptions::PyValueError::new_err)
+}
+
+/// Build a typed :class:`SymbolTable` from a flat dotted-key value map and an
+/// optional per-key EXPR type-spec map, coercing string values to their
+/// declared type and nesting dotted keys (``"Param.Frame"``) into subtables.
+///
+/// ``values`` maps dotted symbol names to their (typically string) values, as
+/// the v0 model stores them. ``types`` maps the same dotted names to EXPR
+/// type spec strings (``"int"``, ``"list[int]"``, ``"path"``, …); names absent
+/// from ``types`` are inferred from the value. ``path_format`` controls how
+/// PATH-typed values are interpreted (defaults to the host OS).
+#[cfg_attr(
+    feature = "stub-gen",
+    gen_stub_pyfunction(module = "openjd._openjd_rs")
+)]
+#[pyfunction]
+#[pyo3(signature = (values, types=None, *, path_format=None))]
+pub(crate) fn build_symbol_table(
+    values: &Bound<'_, PyDict>,
+    types: Option<&Bound<'_, PyDict>>,
+    path_format: Option<PyPathFormat>,
+) -> PyResult<PySymbolTable> {
+    let pf = path_format
+        .map(PathFormat::from)
+        .unwrap_or_else(PathFormat::host);
+    let mut st = SymbolTable::new();
+    for (key, value) in values.iter() {
+        let dotted: String = key.extract()?;
+        let target: Option<ExprType> = match types {
+            Some(t) => match t.get_item(dotted.as_str())? {
+                Some(spec_obj) => {
+                    let spec: String = spec_obj.extract()?;
+                    Some(ExprType::parse(&spec).map_err(pyo3::exceptions::PyValueError::new_err)?)
+                }
+                None => None,
+            },
+            None => None,
+        };
+        let ev = coerce_symbol_value(&value, target.as_ref(), pf)?;
+        st.set(&dotted, ev)
+            .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string()))?;
+    }
+    Ok(PySymbolTable { inner: st })
+}

rust-bindings/src/lib.rs

@@ -81,6 +81,7 @@ fn openjd_rs(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_function(wrap_pyfunction!(evaluate_expression, m)?)?;
     m.add_function(wrap_pyfunction!(parse_expression, m)?)?;
     m.add_function(wrap_pyfunction!(escape_format_string, m)?)?;
+    m.add_function(wrap_pyfunction!(build_symbol_table, m)?)?;
     m.add_function(wrap_pyfunction!(_reconstruct_expr_value, m)?)?;
     m.add_function(wrap_pyfunction!(_reconstruct_serialized_symtab, m)?)?;
 
@@ -231,6 +232,7 @@ fn openjd_rs(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_function(wrap_pyfunction!(validate_attribute_capability_name, m)?)?;
     m.add_function(wrap_pyfunction!(standard_amount_capability_names, m)?)?;
     m.add_function(wrap_pyfunction!(standard_attribute_capability_names, m)?)?;
+    m.add_function(wrap_pyfunction!(job_parameter_type_expr_spec, m)?)?;
     m.add_function(wrap_pyfunction!(standard_attribute_capabilities, m)?)?;
 
     register_renamed_exception(

rust-bindings/src/model/mod.rs

@@ -72,8 +72,8 @@ pub(crate) use template_types::{
     PyStepScript as PyTemplateStepScript, PyStepTemplate,
 };
 pub(crate) use types::{
-    PyDocumentType, PyJobParameterType, PyJobParameterValue, PyTaskParameterType,
-    PyTaskParameterValue, PyTemplateSpecificationVersion,
+    job_parameter_type_expr_spec, PyDocumentType, PyJobParameterType, PyJobParameterValue,
+    PyTaskParameterType, PyTaskParameterValue, PyTemplateSpecificationVersion,
 };
 pub(crate) use user_interfaces::{
     PyBoolUserInterface, PyFileFilter, PyFloatUserInterface, PyHiddenOnlyUserInterface,

rust-bindings/src/model/types.rs

@@ -304,6 +304,22 @@ impl PyJobParameterType {
     }
 }
 
+/// Map an OpenJD job-parameter type spec name (e.g. ``"INT"``, ``"LIST[INT]"``,
+/// ``"RANGE_EXPR"``; case-insensitive) to its EXPR type spec string
+/// (``"int"``, ``"list[int]"``, ``"range_expr"``), or ``None`` when the name is
+/// not a recognized job-parameter type. Single-sources both the
+/// (case-insensitive) type-name parsing and the OpenJD-type -> EXPR-type
+/// mapping in the Rust ``openjd-model`` crate so the Python model does not
+/// hand-maintain a parallel table.
+#[cfg_attr(
+    feature = "stub-gen",
+    gen_stub_pyfunction(module = "openjd._openjd_rs")
+)]
+#[pyfunction]
+pub(crate) fn job_parameter_type_expr_spec(type_name: &str) -> Option<String> {
+    JobParameterType::from_spec_str(type_name.trim()).map(|t| t.expr_type().to_string())
+}
+
 impl From<PyJobParameterType> for JobParameterType {
     fn from(v: PyJobParameterType) -> Self {
         match v {

src/openjd/_openjd_rs.pyi

@@ -1780,6 +1780,12 @@ class ParsedExpression:
     @property
     def expr(self) -> builtins.str: ...
     def __repr__(self) -> builtins.str: ...
+    def typecheck(
+        self,
+        *,
+        values: typing.Optional[typing.Any] = None,
+        profile: typing.Optional[ExprProfile] = None,
+    ) -> None: ...
     def evaluate(
         self,
         *,
@@ -3567,6 +3573,15 @@ def deserialize_step(step_dict: dict) -> Step:
     """
 
 def escape_format_string(value: builtins.str) -> builtins.str: ...
+def job_parameter_type_expr_spec(
+    type_name: builtins.str,
+) -> typing.Optional[builtins.str]: ...
+def build_symbol_table(
+    values: builtins.dict,
+    types: typing.Optional[builtins.dict] = None,
+    *,
+    path_format: typing.Optional[PathFormat] = None,
+) -> SymbolTable: ...
 def evaluate_expression(
     expr: builtins.str,
     *,

src/openjd/model/_create_job.py

@@ -26,6 +26,12 @@
 
 __all__ = ("preprocess_job_parameters",)
 
+# The original scalar job-parameter type names whose values are carried as
+# strings through preprocessing. EXPR-extension types (BOOL, RANGE_EXPR, and
+# the LIST[*] variants) are carried natively instead so the typed EXPR symbol
+# table can coerce them.
+_LEGACY_SCALAR_TYPE_NAMES = frozenset({"STRING", "INT", "FLOAT", "PATH"})
+
 
 # =======================================================================
 # ================ Preprocessing Job Parameters =========================
@@ -70,8 +76,18 @@ def _collect_defaults_2023_09(
     return_value: JobParameterValues = dict[str, ParameterValue]()
     # Collect defaults
     for param in job_parameter_definitions:
+        is_legacy_scalar = param.type.name in _LEGACY_SCALAR_TYPE_NAMES
         if param.name not in job_parameter_values:
             if param.default is not None:
+                if not is_legacy_scalar:
+                    # EXPR types (BOOL / RANGE_EXPR / LIST[*]): carry the native
+                    # default through so the typed symbol-table builder can
+                    # coerce it. The PATH-relative-default handling below only
+                    # applies to the scalar PATH type.
+                    return_value[param.name] = ParameterValue(
+                        type=ParameterValueType(param.type), value=param.default
+                    )
+                    continue
                 default = str(param.default)
                 # Make PATH defaults relative to job_template_dir, and
                 # enforce the `allow_job_template_dir_walk_up` parameter request.
@@ -104,6 +120,12 @@ def _collect_defaults_2023_09(
         else:
             # Check the parameter against the constraints
             value = job_parameter_values[param.name]
+            if not is_legacy_scalar:
+                # EXPR types: carry the provided native value through.
+                return_value[param.name] = ParameterValue(
+                    type=ParameterValueType(param.type), value=value
+                )
+                continue
             # Join any provided relative PATH parameter value with the current_working_directory (except the empty value "")
             if param.type.name == "PATH" and value != "" and not Path(value).is_absolute():
                 value = str(current_working_dir / value)
@@ -123,8 +145,16 @@ def _check_2023_09(
     for param in job_parameter_definitions:
         if param.name in job_parameter_values:
             param_value = job_parameter_values[param.name]
+            # The EXPR-extension LIST[*]/RANGE_EXPR definitions don't implement
+            # _check_constraints (BOOL and the original scalars do). Their
+            # template defaults are validated at decode time, and their values
+            # are type-checked when coerced into the typed EXPR symbol table, so
+            # skip the create-time constraint check when it isn't available.
+            check_constraints = getattr(param, "_check_constraints", None)
+            if check_constraints is None:
+                continue
             try:
-                param._check_constraints(param_value.value)
+                check_constraints(param_value.value)
             except ValueError as err:
                 errors.append(str(err))
 
@@ -312,14 +342,21 @@ def create_job(
     if job_template.specificationVersion == TemplateSpecificationVersion.JOBTEMPLATE_v2023_09:
         from .v2023_09 import ValueReferenceConstants as ValueReferenceConstants_2023_09
 
+        # EXPR-extension typed params (BOOL / RANGE_EXPR / LIST[*]) carry native
+        # values; record their OpenJD type so the typed symbol-table builder
+        # coerces them to the right ExprType during expression evaluation. The
+        # original scalar types keep their existing string-based handling.
+        expr_types: dict[str, str] = {}
         for name, param in all_job_parameter_values.items():
+            prefix = ValueReferenceConstants_2023_09.JOB_PARAMETER_PREFIX.value
+            raw_prefix = ValueReferenceConstants_2023_09.JOB_PARAMETER_RAWPREFIX.value
             if param.type != "PATH":
-                symtab[f"{ValueReferenceConstants_2023_09.JOB_PARAMETER_PREFIX.value}.{name}"] = (
-                    all_job_parameter_values[name].value
-                )
-            symtab[f"{ValueReferenceConstants_2023_09.JOB_PARAMETER_RAWPREFIX.value}.{name}"] = (
-                all_job_parameter_values[name].value
-            )
+                symtab[f"{prefix}.{name}"] = all_job_parameter_values[name].value
+            symtab[f"{raw_prefix}.{name}"] = all_job_parameter_values[name].value
+            if param.type.name not in _LEGACY_SCALAR_TYPE_NAMES:
+                expr_types[f"{prefix}.{name}"] = param.type.value
+                expr_types[f"{raw_prefix}.{name}"] = param.type.value
+        symtab.expr_types.update(expr_types)
     else:
         raise NotImplementedError(
             f"Spec version {job_template.specificationVersion} not implemented."