feat: Implement WRAP_ACTIONS with EXPR binding to RS#285
Conversation
| bindings. This module is the thin bridge between the two. | ||
|
|
||
| Design reference: | ||
| ``SuperDaveDocs/.../delivery-estimate-v3/rs/python-model-with-rust-expr``. |
There was a problem hiding this comment.
Clean up - this is not needed in comment.
| # Errors raised by the Rust expr engine. All subclass ValueError but are | ||
| # distinct classes from the model's own ExpressionError, so they must be | ||
| # caught explicitly at the binding boundary and re-raised as model errors. | ||
| _RUST_EXPR_ERRORS = ( |
There was a problem hiding this comment.
Do the errors here match the PyO3 errors surfaced up by openjd model _v1 namespace - they should match 1:1
| if expr_type is None: | ||
| # Unknown/aggregate type (e.g. LIST[*], RANGE_EXPR) — let the engine | ||
| # infer from the native value for now. Typed list/range coercion is | ||
| # follow-up work (RFC 0007 parameter types). |
There was a problem hiding this comment.
Why? I thought it was implemented.
| # follow-up work (RFC 0007 parameter types). | ||
| return value | ||
| if expr_type == "path": | ||
| return ExprValue(str(value), type=ExprType("path"), path_format=path_format) |
There was a problem hiding this comment.
Do we have equivalent test cases compared to openjd-rs for the path part.
| """ | ||
| types = types or {} | ||
| nested: dict[str, Any] = {} | ||
| for dotted_name in symtab.symbols: |
There was a problem hiding this comment.
Improvement - can we implement this in rust.
| try: | ||
| return parse_expression(expr.strip()) | ||
| except _RUST_EXPR_ERRORS as exc: | ||
| raise _ModelExpressionError(str(exc)) |
There was a problem hiding this comment.
Is this exception 1:1 the same as the Rust version?
| scalar = _OPENJD_TYPE_TO_EXPR_TYPE.get(t) | ||
| if scalar is not None: | ||
| return scalar | ||
| if t.startswith("LIST[") and t.endswith("]"): |
There was a problem hiding this comment.
This seems fragile, can we add a constant?
| return None | ||
|
|
||
|
|
||
| def longest_defined_prefix(name: str, defined: set) -> Optional[str]: |
There was a problem hiding this comment.
Make sure any function not in a _ prefixed file name has proper function prefixes with _ so we do not leak any public functions.
| return f"FullName({self.name})" | ||
|
|
||
|
|
||
| class ExprNode(Node): |
There was a problem hiding this comment.
Is the coding style to embed all the nodes in one file? Or should this be in another dedicated file?
| return f"FullName({self.name})" | ||
|
|
||
|
|
||
| class ExprNode(Node): |
There was a problem hiding this comment.
This node only creates if EXPR and WRAP_ACTIONS is defined in the template ? Check this.
| # unknown-typed symbol (e.g. a `let` name) safely falls back to the | ||
| # name-only check below rather than risking a wrong-type rejection. | ||
| if symbol_types and accessed: | ||
| from ._expr_support import ( |
There was a problem hiding this comment.
Move these imports to init so its in 1 place of failure.
| from ._edit_distance import closest | ||
|
|
||
| name = sorted(missing)[0] | ||
| msg = f"Variable {name} does not exist at this location." |
There was a problem hiding this comment.
Are these error messages 1:1 the same as rust? make sure it is the same for seamless migration.
|
|
||
| def evaluate(self, *, symtab: SymbolTable, path_format: Any = None) -> Any: | ||
| from ._expr_support import ( | ||
| ExprProfile, |
There was a problem hiding this comment.
move import to init.
| """ | ||
| if EXPR_EXTENSION in context.extensions: | ||
| # Raises: model ExpressionError on parse failure. | ||
| return ExprNode(expr) |
There was a problem hiding this comment.
That was an easy integration!
| binding_loc = (*loc, "let", index) | ||
| if name in enclosing: | ||
| errors.append( | ||
| InitErrorDetails( |
There was a problem hiding this comment.
Do these errors match openjd-rs 1:1
There was a problem hiding this comment.
Do we still need to further make sure rust/python errors conform ?
This can be a later thing, just curious about your own comment. Seems like there's some gaps ?
| ArgListType = Annotated[list[ArgString], Field(min_length=1)] | ||
|
|
||
| # WRAP_ACTIONS (RFC 0008) wrap-hook field names on EnvironmentActions. | ||
| _WRAP_ACTION_FIELDS = ("onWrapEnvEnter", "onWrapTaskRun", "onWrapEnvExit") |
There was a problem hiding this comment.
Are constants declared inline across the model generally?
|
|
||
| if any_wrap: | ||
| if "WRAP_ACTIONS" not in extensions: | ||
| raise ValueError( |
There was a problem hiding this comment.
Make sure the error message is the same as openjd-rs
| "require the WRAP_ACTIONS extension." | ||
| ) | ||
| if "EXPR" not in extensions: | ||
| raise ValueError("The WRAP_ACTIONS extension requires the EXPR extension.") |
There was a problem hiding this comment.
Make sure all error messages match openjd-rs
| NODE = "node" | ||
|
|
||
|
|
||
| LET_MAX_BINDINGS = 50 |
There was a problem hiding this comment.
Does this match the openjd spec?
0b88342 to
dd15e34
Compare
a8663e8 to
ac132c1
Compare
| 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, |
There was a problem hiding this comment.
PR #285 — EXPR / WRAP_ACTIONS: key call stacks
Repo: OpenJobDescription/openjd-model-for-python · branch python-rs
What the PR does: teaches the pure-Python (v0) model to handle the RFC
0005–0008 expression extensions by delegating {{ }} parsing, type-checking,
and evaluation to the Rust openjd-expr engine over PyO3 bindings.
TL;DR
Before this PR the pure-Python model only understood the legacy
Name.Dot.Name interpolation grammar. This PR makes it a thin front-end over
the Rust expression engine whenever a template declares the EXPR extension,
and adds the parameter types and wrap-action rules that ride on top of it.
- EXPR expressions — when
EXPRis declared,{{ }}parsing/eval routes to
a new Rust-backedExprNode(arithmetic, functions, lists, paths,let,
comprehensions) instead of the legacy name parser. Non-EXPR templates are
byte-for-byte unchanged, and the Rust surface is imported lazily so the
non-EXPR path never loads it. - Type-aware validation — job-parameter types are mapped to EXPR types and
fed to a newParsedExpression.typecheck()that catches real type errors at
decode time without false-rejecting runtime-dependent expressions (no more
error-string sniffing). - 8 new job-parameter types (RFC 0007) —
BOOL,RANGE_EXPR, and the
LIST[*]family — with case-insensitive type names, item/length constraints,
and an EXPR-extension gate, all sharing one base class. - Native values end-to-end —
ParameterValue.value/JobParameter.value
widen fromstrtoAnyso lists/bools survivecreate_jobintact; a new
SymbolTable.expr_typesfield carries the type tags so the Rust builder
coerces correctly (and is preserved across copy/union). - Spec-correct string coercion — format-string output now uses the engine's
RFC 0005 coercion viaevaluate_to_str()({{ true }}→"true", null →
"", quoted list items) instead of Python'sstr(). - WRAP_ACTIONS (RFC 0008) —
onWrapEnvEnter/onWrapTaskRun/onWrapEnvExiton
EnvironmentActions, gated onWRAP_ACTIONS+EXPR, all-or-nothing, with
per-hookWrapped*variable scoping and a job-wide single-wrap-layer rule. - New Rust bindings —
build_symbol_table,ParsedExpression.typecheck,
job_parameter_type_expr_spec(single-sources the OpenJD→EXPR type mapping).
Risk shape: almost all behavior change is gated behind the EXPR /
WRAP_ACTIONS extension declarations, so templates that don't opt in are
unaffected. The sharp edges are at the Python↔Rust boundary (the three flows
that cross it) and the str → Any value widening.
Two flows form the spine of the change: (1) decode-time validation of an
expression's symbol references, and (2) runtime resolution of an expression
into the string substituted into a format string. ──BOUNDARY──▶ marks a call
crossing from Python into compiled Rust; ◀── marks the return.
Flow 1 — Decode-time: validate an EXPR {{ }} expression's symbols
Entry: decode_job_template(...) → pydantic validation → the variable-reference
prevalidator.
Flow: prevalidate_model_template_variable_references(values, context)
1. _variable_reference_validation.py:137 prevalidate_model_template_variable_references
2. :166 expr_enabled = "EXPR" in declared & in context.extensions
3. :173 context._symbol_types = _build_job_parameter_symbol_types(values)
4. └─ :433 _build_job_parameter_symbol_types
5. └─ _expr_support.py expr_type_for_openjd_type("LIST[INT]")
6. └──BOUNDARY──▶ _openjd_rs job_parameter_type_expr_spec (model/types.rs)
7. uppercase-parse + OpenJD→EXPR map ◀── "list[int]" | None
8. :460 _check_format_string(value, scope, symbols, context)
9. :484 expr.validate_symbol_refs(symbols, symbol_types=ctx._symbol_types)
10. └─ _expression.py InterpolationExpression → self._expresion_tree.validate_symbol_refs
11. └─ _nodes.py:149 ExprNode.validate_symbol_refs(symbols, symbol_types)
12. :150 accessed = self._parsed.accessed_symbols (free dotted refs, from Rust)
13. :167 for name: longest_defined_prefix(name, symbols) → prefix_types
14. :173 ┌── fully typed ── validate_typed_expression(self._parsed, typed_symbols=prefix_types)
15. │ └─ _expr_support.py values = {dotted: ExprValue.unresolved(ExprType(t))}
16. │ └──BOUNDARY──▶ _openjd_rs ParsedExpression.typecheck (parsed_expression.rs)
17. │ eval vs unresolved placeholders, DISCARD result,
18. │ raise ONLY on a genuine type error ◀──
19. └── else (some prefix untyped) ─ name-only fallback:
20. :182 missing = accessed - symbols
21. :184 if missing: raise _missing_symbol_error(name, symbols)
Key insight: step 16's typecheck() discards the result, so a "well-typed
but unresolved at runtime" expression passes cleanly — the model no longer has
to sniff Rust error-message strings to tell a real type error from a
runtime-dependent one. (The let-binding path reaches the same
ExprNode.validate_symbol_refs from _variable_reference_validation.py:606,
building one ExprNode per binding.)
Flow 2 — Runtime: resolve a {{ }} expression to its substituted string
Entry: a caller invokes FormatString.resolve(symtab=...) during job
instantiation / argument resolution.
Flow: FormatString.resolve(symtab, path_format) — {{ }} → substituted string
1. _format_string.py:79 FormatString.resolve(symtab, path_format)
2. :115 element.expression.evaluate_to_str(symtab, path_format)
3. └─ _expression.py InterpolationExpression → self._expresion_tree.evaluate_to_str
4. └─ _nodes.py:234 ExprNode.evaluate_to_str(symtab, path_format)
5. :241 / :208 value = self._evaluate_raw(symtab, path_format)
6. └─ :221 values = symtab_to_expr_values(symtab, types=symtab.expr_types or None)
7. └─ _expr_support.py map OpenJD types → EXPR specs (Flow 1, steps 5–7)
8. └──BOUNDARY──▶ _openjd_rs build_symbol_table(flat, expr_types, path_format)
9. coerce "10":INT → int, nest "Param.Frame" ◀── typed SymbolTable
10. :225 └──BOUNDARY──▶ _openjd_rs ParsedExpression.evaluate(values, profile, path_format) ◀── ExprValue
11. :242 if value.is_null: return "" # null → empty string (RFC 0005)
12. :244 return str(value) # ExprValue.__str__ = engine spec coercion
13. :127 resolved_list.append(resolved_value) # NOTE: str() wrapper removed here
Key insight: step 12's str(ExprValue) is the fix for the
{{ true }} → "True" Python-repr bug — it uses the engine's RFC 0005 coercion
(true/false, double-quoted list items, preserved Decimal trailing zeros),
not Python's str(). Step 9 is where the expr_types metadata earns its keep:
if that map is ever dropped on a SymbolTable copy/union, coercion silently
reverts to inference and produces a wrong-typed value — which is exactly why the
PR promotes expr_types to a real field that is copied in __init__ and
union. ExprNode.evaluate (_nodes.py:231) instead calls .item() to hand
back the native Python value for callers that want the typed result.
Flow 3 — Decode-time: a new typed job parameter (BOOL / LIST[*] / RANGE_EXPR)
The PR adds eight EXPR-extension job-parameter types. They share one base class
and a normalize-then-discriminate decode path. Entry: decode_job_template(...)
on a template whose parameterDefinitions contains, say, {"type": "list[int]"}.
Flow: JobTemplate.parameterDefinitions validation
1. _model.py:3809 @field_validator("parameterDefinitions", mode="before")
2. └─ :3427 _normalize_parameter_type_case(value, info)
3. :3434 if "EXPR" in context.extensions: item["type"] = item["type"].upper()
4. # "list[int]" → "LIST[INT]" (RFC 0007 case-insensitive type names)
5. │ (pydantic now resolves the discriminated union on the upper-cased "type")
6. └─ Literal[JobParameterType.LIST_INT] ─▶ JobListIntParameterDefinition
7. └─ class _JobListParameterDefinitionBase (_model.py:3477)
8. :3487 @field_validator("type") _validate_type_gate → _expr_param_gate
9. if context & "EXPR" not in extensions: raise "requires the EXPR extension"
10. :3519 @model_validator(after) _validate_default
11. └─ :3512 _check_list_value(self.default)
12. _check_list_length(name, v, minLength, maxLength)
13. for item: self._check_item(item) # subclass hook
14. └─ JobListIntParameterDefinition._check_item → _check_int_item
15. └─ _check_numeric_item_bounds(...) # shared int/float bounds (deduped this PR)
Key insight: every concrete list type is just a type literal + an item
constraint + a one-line _check_item override on the shared base — the length
check, EXPR gate, create-job metadata, and template-variable defs all live once
on _JobListParameterDefinitionBase. RANGE_EXPR (_model.py:3644) is the odd
one out: it validates its default string through the existing IntRangeExpr
grammar rather than a list check. Two-stage decode (normalize before,
discriminate after) is why a lowercase list[int] resolves to the right
class instead of failing the union.
Flow 4 — Instantiation: create_job carries native values for the new types
Legacy scalars (STRING/INT/FLOAT/PATH) are stringified through preprocessing;
the new EXPR types must keep their native Python value (a real list/bool) so
the typed symbol-table builder (Flow 2, step 8) can coerce them. Entry:
create_job(job_template, job_parameter_values).
Flow: create_job(template, values)
1. _create_job.py:293 create_job
2. :64 _collect_defaults_2023_09
3. :79 is_legacy_scalar = param.type.name in _LEGACY_SCALAR_TYPE_NAMES
4. :82 ┌─ default ─ if not is_legacy_scalar: ParameterValue(value=param.default) # native, no str()
5. :123 └─ provided ─ if not is_legacy_scalar: ParameterValue(value=value) # native, no str()
6. (legacy branch still does str(value) + PATH-relative join below)
7. :139 _check_2023_09(defs, values)
8. :155 check_constraints = getattr(param, "_check_constraints", None)
9. if check_constraints: check_constraints(param_value.value) # list/range checked here
10. :349 expr_types = {}; for each non-legacy param:
11. :357 expr_types["Param.<n>"] = expr_types["RawParam.<n>"] = param.type.value
12. :359 symtab.expr_types.update(expr_types) # ← feeds Flow 2 step 8 coercion
Key insight: this is the producer side of the expr_types contract that
Flow 2 consumes. The widened ParameterValue.value: Any (was str) is what lets
a [1, 2, 3] survive instantiation intact; the review confirmed no downstream
consumer string-ops that value, and the type tag travels alongside it via
symtab.expr_types so the Rust builder coerces correctly.
Merge wrinkle — _merge_job_parameter.py
When the same parameter is defined in multiple templates, only the four legacy
scalar types take the cross-merge path (merge_job_parameter_definitions_for_one
:191, gated on _LEGACY_CONSTRAINT_TYPES). EXPR types skip it and return the
last-defined definition via model_copy (:241) — which bypasses validators, so
the PR re-runs getattr(base, "_check_constraints", None) (:235) on the carried
default to avoid letting a constraint-violating default slip through.
Flow 5 — Decode-time: WRAP_ACTIONS structural validation (RFC 0008)
Two independent checks fire when WRAP_ACTIONS is declared: per-EnvironmentActions
shape/scope, and a job-wide single-wrap-layer rule.
Flow: EnvironmentActions + JobTemplate WRAP_ACTIONS validation
A. _model.py:516 EnvironmentActions._requires_oneof (mode=before)
any_wrap = any of (onWrapEnvEnter/onWrapTaskRun/onWrapEnvExit) set
if any_wrap & context present:
require "WRAP_ACTIONS" in extensions; require "EXPR" (hard prereq)
require ALL THREE wrap hooks (all-or-nothing)
# context-gated: skipped at create_job re-validation (no context)
B. _model.py:565 EnvironmentActions._validate_wrapped_variable_scope (mode=after)
for field in (onEnter, onExit, *wrap hooks):
referenced = _action_referenced_namespaces(action) # :398, scans command/args/timeout FormatStrings
└─ expr.accessed_symbols → namespace = symbol.split(".")[0] # WrappedAction/WrappedEnv/WrappedStep
reject namespaces not allowed in that hook (WrappedStep only in onWrapTaskRun, etc.)
C. _model.py:3941 JobTemplate._validate_single_wrap_layer (mode=after)
job_env_wraps = count of jobEnvironments with a wrap hook (_env_defines_wrap_hook :373)
for each step: if job_env_wraps + step's stepEnvironment wraps > 1: reject
# "at most one wrap layer reachable per session stack"
Key insight: the three checks live at different altitudes on purpose —
extension/all-or-nothing gating per EnvironmentActions (before), per-hook
variable scoping per EnvironmentActions (after), and the cross-cutting
single-layer count at JobTemplate (after, where it can see jobEnvironments +
all steps). The scope check (B) deliberately inspects the timeout FormatString
too, not just command/args, because timeout is a format string under
FEATURE_BUNDLE_1.
Reference — type changes that ripple through the flows
| Change | File | Why it matters |
|---|---|---|
ParameterValue.value: str → Any |
_types.py |
lets BOOL/LIST values ride through preprocessing natively (Flow 4); a stringified list can't be re-parsed into a typed list |
JobParameter.value: str → Any |
v2023_09/_model.py |
same, on the instantiated-job side |
SymbolTable.expr_types: dict[str,str] (new field) |
_symbol_table.py |
the type-tag map; copied in __init__/union so coercion (Flow 2 step 8) never silently reverts to inference |
ExpressionInfo.resolved_value: Optional[Union[Real,str]] → Optional[str] |
_format_string.py |
resolve() now appends the value directly (str() wrapper dropped); evaluate_to_str guarantees a string |
Node.allows_nonscalar + evaluate_to_str() (new) |
_nodes.py |
lets EXPR nodes return list/bool/null while the legacy FullNameNode stays str/Real-only |
validate_symbol_refs(..., symbol_types=...) (new kwarg) |
_nodes.py / _expression.py |
threads the type map into the typed-validation fork (Flow 1 step 11) |
DefinesTemplateVariables.expr_inject (new) |
_types.py |
EXPR-only symbol injection (e.g. Step.Name) applied by the prevalidator, which has the parsing context |
New Rust bindings: build_symbol_table, ParsedExpression.typecheck, job_parameter_type_expr_spec |
rust-bindings/ |
the three boundary calls the flows above cross into |
|
I think in general things look good. I pulled the changes and tested on my own, nothing significant that I think needs changing, one comment about existing comment |
|
also double checking, would there be a good e2e way to test sessions model together ? |
jericht
left a comment
jericht
left a comment
There was a problem hiding this comment.
Looks good overall, we can iterate on any small issues in future PRs
Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
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>
Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
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>
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>
…alidation 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>
… 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>
Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
3 participants
Fixes:
What was the problem/requirement? (What/Why)
The pure-Python (v0) model did not support the EXPR extension (RFCs 0005–0007) or the WRAP_ACTIONS extension (RFC 0008). As a result it diverged from the
openjd-rsreference implementation and from the published specification:onWrapEnvEnter/onWrapTaskRun/onWrapEnvExit) could not be parsed or validated by the Python model.openjd-sessions-for-python) had no way to obtain the wrap-hook model or the typed EXPR symbol-table machinery they depend on.What was the solution? (How)
openjd-exprengine through theopenjd._openjd_rsPyO3 bindings, instead of reimplementing the expression grammar in Python.build_symbol_table— typed, dotted-key EXPR symbol tables (string→typed coercion lives next to the engine).job_parameter_type_expr_spec— single-sourced OpenJD-type → EXPR type-spec mapping (includingLIST[...]nesting), replacing a hand-maintained Python table.ParsedExpression.typecheck— typed static validation without result extraction (removes brittle error-string sniffing).openjd-rsvalidators:WrappedAction.*/WrappedEnv.*/WrappedStep.*variable scoping.mypyerrors so the package type-checks cleanly.What is the impact of this change?
The Python model now accepts and validates EXPR (RFC 0005–0007) and WRAP_ACTIONS (RFC 0008) templates consistently with the
openjd-rsreference and the specification. Expression parsing/evaluation and the OpenJD-type → EXPR-type mapping are single-sourced in Rust, eliminating Python/Rust drift. This unblocks downstream consumers (notablyopenjd-sessions-for-python) that require the wrap-hook model and the EXPR bindings.How was this change tested?
See DEVELOPMENT.md for information on running tests.
The full unit and conformance test suite passes: 5273 passed, 24 skipped, 6 xfailed (the 6
xfails are pre-existing, documented known gaps, not regressions). All conformance fixtures pass, including the WRAP_ACTIONS cases. Static checks are also clean:ruff check,black --check, andmypy(105 source files) all pass.Was this change documented?
_expr_supportRust-bridge module, the new binding methods, and the wrap-rule validators reference the corresponding RFCs). No user-facing schema/doc changes were required beyond code-level documentation.Is this a breaking change?
A breaking change is one that modifies a public contract in a way that is not backwards compatible. See the Public Interfaces section of the DEVELOPMENT.md for more information on the public contracts.
No. The changes are additive and opt-in: EXPR and WRAP_ACTIONS behavior is enabled only when those extensions are declared on a template. Existing (non-EXPR) templates parse and validate exactly as before. Public type aliases were widened (never narrowed), so existing callers remain compatible.
Does this change impact security?
openjd-exprengine (bounded memory, no arbitrary code execution) and only when the EXPR extension is declared. This change does not create or modify any files/directories that must be readable only by the process owner.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.