From 202397ecd742664fcb31f7167eadddaa542e49e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 02:03:31 +0000 Subject: [PATCH 1/2] Reuse the cel environment built from a Context across evaluations Every evaluate() and Program.execute() call rebuilt the cel-rust context from scratch: each variable was re-boxed and each registered Python function was re-wrapped in a closure, so the per-call cost scaled with the size of the Python Context rather than the expression. Executing a pre-compiled `1 + 2` against a Context carrying the 47 extended-stdlib functions (the CLI's setup) cost ~7.5 us against ~0.13 us with no context; a Context with 200 variables cost ~32 us. The Context now builds that environment on first use and caches it behind a Mutex as an Arc>. add_variable, add_function and update drop the cache; set_variable_resolver does not need to, because the resolver is bound per call in a child scope (cel-rust's Context::new_inner_scope), which keeps the same lookup order as before (resolver, then registered variables). Handing out an Arc rather than borrowing the Python object means a callback may mutate, or re-enter evaluation with, the Context it is registered on; the mutation applies from the next evaluation. Dict contexts are still built per call, since a dict can change without notice. Both cases above now execute in ~0.15 us. The dict and no-context paths are unchanged within noise. Also factors the Python-function wrapper and the compile step into helpers shared by evaluate() and compile(), and corrects the panic message for an execution-time panic, which called itself a parser error. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW --- CHANGELOG.md | 28 +++ docs/reference/python-api.md | 6 + src/context.rs | 59 +++++- src/lib.rs | 361 ++++++++++++++++++----------------- tests/test_context_reuse.py | 166 ++++++++++++++++ 5 files changed, 439 insertions(+), 181 deletions(-) create mode 100644 tests/test_context_reuse.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7786f0b..f54b729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Performance + +- **A `cel.Context` now builds its CEL-side environment once and reuses it.** + Previously every `evaluate()` and `Program.execute()` call re-converted each + variable and re-wrapped each registered Python function into a fresh cel-rust + context, so the per-call cost grew with the size of the context: executing a + pre-compiled `1 + 2` against a `Context` carrying the 47 extended-stdlib + functions (what the `cel` CLI sets up) took about 9 µs, against about 0.2 µs + with no context. The environment is now cached on the `Context` and shared by + every evaluation until `add_variable`, `add_function` or `update` changes it, + bringing that case down to about 0.2 µs as well. `set_variable_resolver` does + not invalidate the cache: the resolver is bound per call in a child scope + (cel-rust's `Context::new_inner_scope`), with the same lookup order as before + (resolver, then registered variables). Dict contexts are still materialised on + every call, since a dict can change between calls without notice; for hot + loops, prefer a `Context`. +- Behaviour that is unchanged and now pinned by tests: every mutator is visible on + the next evaluation; a Python function may modify, or evaluate against, the + `Context` it was registered on while an evaluation is in progress (the change + applies from the next evaluation); and one `Context` can be shared between + threads. + +### Changed + +- The `ValueError` raised when the interpreter panics during evaluation now reads + "Internal evaluation error" rather than "Internal parser error"; parse failures + were never what it reported. + ## [0.9.0] - 2026-09-09 Upgrades to cel-rust 0.14.5, which brings native `type()`, range-checked diff --git a/docs/reference/python-api.md b/docs/reference/python-api.md index bb9892b..cd236ae 100644 --- a/docs/reference/python-api.md +++ b/docs/reference/python-api.md @@ -207,6 +207,12 @@ The Context class provides more control over the evaluation environment than sim - Register custom Python functions - Manage complex evaluation scenarios +It is also the fast path for repeated evaluation. A `Context` converts its +variables and wraps its functions for the CEL engine once, on first use, and +reuses that work for every subsequent `evaluate()` or `Program.execute()` call +until the context is modified. A dict passed as the context is converted afresh +on every call, because it can change between calls without notice. + ```python from cel import evaluate, Context diff --git a/src/context.rs b/src/context.rs index 76e63e5..a5a59db 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,9 +1,10 @@ use ::cel::objects::TryIntoValue; -use ::cel::Value; +use ::cel::{Context as CelContext, Value}; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; use pyo3::types::PyDict; use std::collections::HashMap; +use std::sync::{Arc, Mutex, PoisonError}; #[pyo3::pyclass] /// Manages the evaluation environment for CEL expressions. @@ -37,15 +38,63 @@ use std::collections::HashMap; /// for concurrent use or implement your own synchronization. /// /// Performance Tips: -/// - Reuse Context objects for multiple evaluations when possible +/// - Reuse Context objects for multiple evaluations when possible: the +/// CEL-side environment (converted variables and wrapped functions) is +/// built on first use and reused until the context is modified /// - Pre-populate Context with all needed variables and functions -/// - Avoid frequent add_variable/add_function calls in hot code paths +/// - Avoid frequent add_variable/add_function calls in hot code paths, as +/// each one discards the cached environment pub struct Context { pub variables: HashMap, pub functions: HashMap>, /// Optional Python callable for lazy variable resolution. Invoked with a /// variable name; returns the value (or None to fall through to `variables`). pub resolver: Option>, + /// The cel environment built from `variables` and `functions`, created on + /// first use and shared by every evaluation until a mutator clears it. + /// + /// Building it boxes each variable and wraps each Python callable in a + /// closure. That used to happen on every `evaluate()`/`execute()` call and + /// dominated the cost of evaluating against a context with many functions + /// (the CLI registers the whole extended stdlib). The resolver is + /// deliberately not part of it: it is bound per call in a child scope, so + /// setting one does not invalidate the cache. + cel: Mutex>>>, +} + +impl Context { + /// Materialises a fresh cel environment from the registered variables and + /// functions. Used for the cache and, on every call, for dict contexts. + pub(crate) fn build_cel_context(&self, py: Python<'_>) -> CelContext<'static> { + let mut environment = crate::new_environment(); + for (name, value) in &self.variables { + environment.add_variable_from_value(name.clone(), value.clone()); + } + for (name, function) in &self.functions { + crate::register_python_function(&mut environment, name, function.clone_ref(py)); + } + environment + } + + /// Returns the cached cel environment, building it on first use. + /// + /// The `Arc` lets a caller keep evaluating against a consistent snapshot + /// even if a Python callback mutates this `Context` mid-evaluation; the + /// mutation simply takes effect from the next evaluation. + pub(crate) fn cel_context(&self, py: Python<'_>) -> Arc> { + let mut cached = self.cel.lock().unwrap_or_else(PoisonError::into_inner); + if let Some(existing) = cached.as_ref() { + return Arc::clone(existing); + } + let built = Arc::new(self.build_cel_context(py)); + *cached = Some(Arc::clone(&built)); + built + } + + /// Drops the cached environment so the next evaluation rebuilds it. + fn invalidate(&mut self) { + *self.cel.get_mut().unwrap_or_else(PoisonError::into_inner) = None; + } } #[pyo3::pymethods] @@ -126,6 +175,7 @@ impl Context { variables: HashMap::new(), functions: HashMap::new(), resolver: None, + cel: Mutex::new(None), }; if let Some(variables) = variables { @@ -205,6 +255,7 @@ impl Context { /// >>> # Note: This would need proper error handling in practice fn add_function(&mut self, name: String, function: Py) { self.functions.insert(name, function); + self.invalidate(); } /// Registers a Python callable for lazy variable resolution. @@ -318,6 +369,7 @@ impl Context { )) })?; self.variables.insert(name, value); + self.invalidate(); Ok(()) } @@ -427,6 +479,7 @@ impl Context { self.variables.insert(key, value); } } + self.invalidate(); Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index f93f8c3..84897b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,7 +38,7 @@ fn stdlib_env() -> Arc { } /// Builds a fresh execution environment backed by the shared standard library. -fn new_environment() -> CelContext<'static> { +pub(crate) fn new_environment() -> CelContext<'static> { CelContext::with_env(stdlib_env()) } @@ -251,7 +251,17 @@ impl PyOptionalValue { /// 30 #[pyfunction] fn compile(expression: String) -> PyResult { - let program = panic::catch_unwind(|| Program::compile(&expression)) + let program = compile_program(&expression)?; + Ok(PyProgram { + program, + source: expression, + }) +} + +/// Parses `expression`, turning both parse errors and parser panics into +/// `ValueError` so callers can rely on one exception type for a bad expression. +fn compile_program(expression: &str) -> PyResult { + panic::catch_unwind(|| Program::compile(expression)) .map_err(|_| { warn!("CEL parser panic for expression: '{}'", expression); PyValueError::new_err(format!( @@ -260,12 +270,7 @@ fn compile(expression: String) -> PyResult { })? .map_err(|e| { PyValueError::new_err(format!("Failed to parse expression '{expression}': {e}")) - })?; - - Ok(PyProgram { - program, - source: expression, - }) + }) } #[derive(Debug)] @@ -443,138 +448,176 @@ impl VariableResolver for PyVariableResolver { } } -/// Build a CEL execution environment from an optional evaluation context. +/// Registers a Python callable as the CEL function `function_name` on `environment`. /// -/// This consolidates the shared logic used by `evaluate()` and `Program.execute()` -/// to keep behavior consistent between the two entrypoints. -fn build_environment<'r>( - evaluation_context: Option<&Bound<'_, PyAny>>, - environment: &mut CelContext<'r>, - resolver_out: &'r mut Option, -) -> PyResult<()> { - let mut ctx = context::Context::new(None, None)?; - - // Process the evaluation context if provided - if let Some(evaluation_context) = evaluation_context { - // Attempt to extract directly as a Context object - if let Ok(py_context_ref) = evaluation_context.extract::>() { - // Clone variables and functions into our local Context - ctx.variables = py_context_ref.variables.clone(); - ctx.functions = py_context_ref.functions.clone(); - if let Some(cb) = py_context_ref.resolver.as_ref() { - *resolver_out = Some(PyVariableResolver { - callback: Python::attach(|py| cb.clone_ref(py)), - }); +/// The wrapper takes the raw `FunctionContext` (rather than the `Arguments` +/// extractor) so that method-call syntax works: when an expression calls +/// `target.func(a, b)`, CEL puts `target` in `ftx.this` and `[a, b]` in +/// `ftx.args`. Prepending `this` to the argument list means the Python function +/// receives `(target, a, b)`, so a Python function `f(x, y)` can be invoked as +/// either `f(x, y)` or `x.f(y)` — matching CEL's "receiver call is sugar for a +/// function call with the receiver as the first argument" semantics and the way +/// the standard library extensions (e.g. `list.contains(x)`, `"s".charAt(i)`) +/// are written. +pub(crate) fn register_python_function( + environment: &mut CelContext<'_>, + function_name: &str, + py_func: Py, +) { + let func_name = function_name.to_string(); + environment.add_function( + function_name, + move |ftx: &FunctionContext| -> Result { + // Collect the CEL argument values: the method target (if this was a + // receiver-style call) first, then the explicit arguments. + let mut cel_args: Vec = Vec::with_capacity(ftx.args.len() + 1); + if let Some(this) = &ftx.this { + cel_args.push(this.as_ref().try_into()?); + } + for arg in ftx.args.iter() { + cel_args.push(arg.as_ref().try_into()?); } - } else if let Ok(py_dict) = evaluation_context.cast::() { - // User passed in a dict - let's process variables and functions from the dict - ctx.update(py_dict)?; - } else { - return Err(PyValueError::new_err( - "evaluation_context must be a Context object or a dict", - )); - }; - // Add any variables from the processed context. The values are already - // `cel::Value`s, so `add_variable_from_value` (infallible, `Into`) - // is the right entry point — no conversion or error handling needed here. - for (name, value) in &ctx.variables { - environment.add_variable_from_value(name.clone(), value.clone()); - } + Python::attach(|py| { + let mut py_args = Vec::with_capacity(cel_args.len()); + for cel_value in cel_args { + let py_arg = RustyCelType(cel_value) + .into_pyobject(py) + .map_err(|e| ExecutionError::FunctionError { + function: func_name.clone(), + message: format!("Failed to convert argument to Python: {e}"), + })? + .into_any() + .unbind(); + py_args.push(py_arg); + } - // Register Python functions - for (function_name, py_function) in ctx.functions.iter() { - // Create a wrapper function - let py_func_clone = Python::attach(|py| py_function.clone_ref(py)); - let func_name_clone = function_name.clone(); - - // Register a wrapper that bridges the CEL call to the Python callable. - // - // We take the raw `FunctionContext` (rather than the `Arguments` - // extractor) so that method-call syntax works: when an expression - // calls `target.func(a, b)`, CEL puts `target` in `ftx.this` and - // `[a, b]` in `ftx.args`. We prepend `this` to the argument list so - // the Python function receives `(target, a, b)`. This means a Python - // function `f(x, y)` can be invoked as either `f(x, y)` or - // `x.f(y)` — matching CEL's "receiver call is sugar for a function - // call with the receiver as the first argument" semantics and the - // way the standard library extensions (e.g. `list.contains(x)`, - // `"s".charAt(i)`) are written. - environment.add_function( - function_name, - move |ftx: &FunctionContext| -> Result { - let py_func = py_func_clone.clone(); - let func_name = func_name_clone.clone(); - - // Collect the CEL argument values: the method target (if - // this was a receiver-style call) first, then the explicit - // arguments. - let mut cel_args: Vec = Vec::with_capacity(ftx.args.len() + 1); - if let Some(this) = &ftx.this { - cel_args.push(this.as_ref().try_into()?); - } - for arg in ftx.args.iter() { - cel_args.push(arg.as_ref().try_into()?); + let py_args_tuple = + PyTuple::new(py, py_args).map_err(|e| ExecutionError::FunctionError { + function: func_name.clone(), + message: format!("Failed to create arguments tuple: {e}"), + })?; + + let py_result = py_func.call1(py, py_args_tuple).map_err(|e| { + warn!("Python function '{}' failed: {}", func_name, e); + ExecutionError::FunctionError { + function: func_name.clone(), + message: format!("Python function call failed: {e}"), } + })?; - Python::attach(|py| { - // Convert CEL arguments to Python objects - let mut py_args = Vec::with_capacity(cel_args.len()); - for cel_value in cel_args { - let py_arg = RustyCelType(cel_value) - .into_pyobject(py) - .map_err(|e| ExecutionError::FunctionError { - function: func_name.clone(), - message: format!("Failed to convert argument to Python: {e}"), - })? - .into_any() - .unbind(); - py_args.push(py_arg); - } - - let py_args_tuple = PyTuple::new(py, py_args).map_err(|e| { - ExecutionError::FunctionError { - function: func_name.clone(), - message: format!("Failed to create arguments tuple: {e}"), - } - })?; - - // Call the Python function - let py_result = py_func.call1(py, py_args_tuple).map_err(|e| { - warn!("Python function '{}' failed: {}", func_name, e); - ExecutionError::FunctionError { - function: func_name.clone(), - message: format!("Python function call failed: {e}"), - } - })?; + RustyPyType(py_result.bind(py)) + .try_into_value() + .map_err(|e| ExecutionError::FunctionError { + function: func_name.clone(), + message: format!("Failed to convert Python result to CEL value: {e}"), + }) + }) + }, + ); +} - // Convert the result back to CEL Value - let py_result_ref = py_result.bind(py); - let cel_value = - RustyPyType(py_result_ref).try_into_value().map_err(|e| { - ExecutionError::FunctionError { - function: func_name.clone(), - message: format!( - "Failed to convert Python result to CEL value: {e}" - ), - } - })?; +/// The cel environment an evaluation runs against. +/// +/// A dict context is materialised afresh for each call, because a dict can +/// change between calls without telling us. A [`context::Context`] instead +/// hands out the environment it caches, shared through an `Arc` so a Python +/// callback may mutate the `Context` during evaluation without disturbing the +/// evaluation already in flight. +enum Root { + Owned(CelContext<'static>), + Shared(Arc>), +} - Ok(cel_value) - }) - }, - ); +impl Root { + fn as_cel(&self) -> &CelContext<'static> { + match self { + Root::Owned(environment) => environment, + Root::Shared(environment) => environment, } } +} + +/// Everything an evaluation needs from the Python-side context: the cel +/// environment plus the lazy variable resolver, if one is registered. +/// +/// The resolver is kept out of the root and bound per call in a child scope +/// (see [`run_program`]), which is what lets the root be cached and shared. +struct Environment { + root: Root, + resolver: Option, +} - // Attach the lazy resolver if one was provided. The resolver lives in - // `*resolver_out` (caller-owned), and the cel::Context borrows it for - // its lifetime `'r`. - if let Some(resolver) = resolver_out.as_ref() { - environment.set_variable_resolver(resolver); +/// Turns the `evaluation_context` argument of `evaluate()`/`Program.execute()` +/// into an [`Environment`], so the two entry points behave identically. +fn prepare_environment(evaluation_context: Option<&Bound<'_, PyAny>>) -> PyResult { + let Some(evaluation_context) = evaluation_context else { + return Ok(Environment { + root: Root::Owned(new_environment()), + resolver: None, + }); + }; + let py = evaluation_context.py(); + + if let Ok(py_context) = evaluation_context.extract::>() { + // The borrow of the Python object ends when `py_context` drops at the end + // of this block, before any Python callback can run, so a callback that + // mutates the Context mid-evaluation does not hit a "borrowed" error. + let resolver = py_context + .resolver + .as_ref() + .map(|callback| PyVariableResolver { + callback: callback.clone_ref(py), + }); + Ok(Environment { + root: Root::Shared(py_context.cel_context(py)), + resolver, + }) + } else if let Ok(py_dict) = evaluation_context.cast::() { + // A dict mixes variables and functions; `Context::update` sorts them by + // callability exactly as it does for a Python `Context`. + let mut ctx = context::Context::new(None, None)?; + ctx.update(py_dict)?; + Ok(Environment { + root: Root::Owned(ctx.build_cel_context(py)), + resolver: None, + }) + } else { + Err(PyValueError::new_err( + "evaluation_context must be a Context object or a dict", + )) } +} - Ok(()) +/// Executes `program` against `environment`, mapping interpreter panics and +/// execution errors to Python exceptions. +/// +/// A registered resolver is attached to a child scope of the root rather than +/// to the root itself. Lookups in the child consult the resolver first and then +/// fall through to the parent's variables, which is the same order the resolver +/// had when it lived on the root, and the root stays untouched and reusable. +fn run_program(program: &Program, src: &str, environment: &Environment) -> PyResult { + let root = environment.root.as_cel(); + let scoped; + let ctx: &CelContext<'_> = match &environment.resolver { + Some(resolver) => { + let mut scope = root.new_inner_scope(); + scope.set_variable_resolver(resolver); + scoped = scope; + &scoped + } + None => root, + }; + + // AssertUnwindSafe is needed because the environment contains function closures. + let result = panic::catch_unwind(AssertUnwindSafe(|| program.execute(ctx))).map_err(|_| { + warn!("CEL execution panic for expression: '{}'", src); + PyValueError::new_err(format!( + "Failed to execute expression '{src}': Internal evaluation error" + )) + })?; + + result.map_err(|error| map_execution_error_to_python(&error)) } /// Human-readable CEL type name for a value (e.g. `int`, `uint`, `string`). @@ -973,65 +1016,27 @@ impl TryIntoValue for RustyPyType<'_> { /// - Python API Reference: For detailed API documentation #[pyfunction(signature = (src, evaluation_context=None))] fn evaluate(src: String, evaluation_context: Option<&Bound<'_, PyAny>>) -> PyResult { - let mut environment = new_environment(); - let mut resolver_slot: Option = None; - build_environment(evaluation_context, &mut environment, &mut resolver_slot)?; - - // Use panic::catch_unwind to handle parser panics gracefully - let program = panic::catch_unwind(|| Program::compile(&src)) - .map_err(|_| { - warn!("CEL parser panic for expression: '{}'", src); - PyValueError::new_err(format!( - "Failed to parse expression '{src}': Invalid syntax or malformed string" - )) - })? - .map_err(|e| PyValueError::new_err(format!("Failed to parse expression '{src}': {e}")))?; - - // Use panic::catch_unwind to handle execution panics gracefully - // AssertUnwindSafe is needed because the environment contains function closures - let result = - panic::catch_unwind(AssertUnwindSafe(|| program.execute(&environment))).map_err(|_| { - warn!("CEL execution panic for expression: '{}'", src); - PyValueError::new_err(format!( - "Failed to execute expression '{src}': Internal parser error" - )) - })?; - - match result { - Err(error) => Err(map_execution_error_to_python(&error)), - Ok(value) => Ok(RustyCelType(value)), - } + // Validate the context before parsing so a bad context and a bad expression + // report in the same order they always have. + let environment = prepare_environment(evaluation_context)?; + let program = compile_program(&src)?; + run_program(&program, &src, &environment).map(RustyCelType) } /// Internal helper to execute a pre-compiled program with the given context. -/// Used by both `evaluate()` (after compiling) and `PyProgram.execute()`. +/// Used by `PyProgram.execute()`. fn execute_compiled_program( program: &Program, src: &str, evaluation_context: Option<&Bound<'_, PyAny>>, ) -> PyResult> { - let mut environment = new_environment(); - let mut resolver_slot: Option = None; - build_environment(evaluation_context, &mut environment, &mut resolver_slot)?; - - // Use panic::catch_unwind to handle execution panics gracefully - // AssertUnwindSafe is needed because the environment contains function closures - let result = - panic::catch_unwind(AssertUnwindSafe(|| program.execute(&environment))).map_err(|_| { - warn!("CEL execution panic for expression: '{}'", src); - PyValueError::new_err(format!( - "Failed to execute expression '{src}': Internal parser error" - )) - })?; - - match result { - Err(error) => Err(map_execution_error_to_python(&error)), - Ok(value) => Python::attach(|py| { - RustyCelType(value) - .into_pyobject(py) - .map(|obj| obj.unbind()) - }), - } + let environment = prepare_environment(evaluation_context)?; + let value = run_program(program, src, &environment)?; + Python::attach(|py| { + RustyCelType(value) + .into_pyobject(py) + .map(|obj| obj.unbind()) + }) } #[pymodule] diff --git a/tests/test_context_reuse.py b/tests/test_context_reuse.py new file mode 100644 index 0000000..411ecd7 --- /dev/null +++ b/tests/test_context_reuse.py @@ -0,0 +1,166 @@ +"""Behavioural contract for reusing a ``cel.Context`` across evaluations. + +A ``Context`` builds its CEL-side environment (converted variables and wrapped +Python functions) once and reuses it for every ``evaluate()``/``execute()`` +call until it is modified. These tests pin the observable consequences: every +mutator is visible on the next evaluation, a callback may modify the context it +is running under, evaluation is re-entrant, and a single context can be shared +between threads. +""" + +from concurrent.futures import ThreadPoolExecutor + +import cel +import pytest +from cel import Context +from cel.stdlib import add_stdlib_to_context + + +class TestMutationsInvalidateTheCache: + def test_add_variable_after_first_evaluation(self): + ctx = Context({"a": 1}) + program = cel.compile("a + b") + with pytest.raises(RuntimeError, match="Undefined variable"): + program.execute(ctx) + + ctx.add_variable("b", 2) + assert program.execute(ctx) == 3 + + def test_overwrite_variable_after_first_evaluation(self): + ctx = Context({"counter": 1}) + assert cel.evaluate("counter", ctx) == 1 + ctx.add_variable("counter", 2) + assert cel.evaluate("counter", ctx) == 2 + + def test_add_function_after_first_evaluation(self): + ctx = Context({"x": 21}) + assert cel.evaluate("x", ctx) == 21 + with pytest.raises(RuntimeError, match="Undefined variable or function"): + cel.evaluate("twice(x)", ctx) + + ctx.add_function("twice", lambda v: v * 2) + assert cel.evaluate("twice(x)", ctx) == 42 + + def test_replace_function_after_first_evaluation(self): + ctx = Context(functions={"f": lambda: "first"}) + assert cel.evaluate("f()", ctx) == "first" + ctx.add_function("f", lambda: "second") + assert cel.evaluate("f()", ctx) == "second" + + def test_update_after_first_evaluation(self): + ctx = Context({"a": 1}) + assert cel.evaluate("a", ctx) == 1 + ctx.update({"a": 10, "b": 5, "add": lambda x, y: x + y}) + assert cel.evaluate("add(a, b)", ctx) == 15 + + def test_resolver_set_after_first_evaluation(self): + ctx = Context({"static_var": 1}) + assert cel.evaluate("static_var", ctx) == 1 + with pytest.raises(RuntimeError, match="Undefined variable"): + cel.evaluate("dynamic_var", ctx) + + ctx.set_variable_resolver(lambda name: 99 if name == "dynamic_var" else None) + assert cel.evaluate("dynamic_var", ctx) == 99 + # The resolver is consulted first, then registered variables. + assert cel.evaluate("static_var", ctx) == 1 + + def test_resolver_shadows_registered_variable(self): + """The resolver keeps precedence over add_variable() values, as documented.""" + ctx = Context({"x": "static"}) + ctx.set_variable_resolver(lambda name: "resolved" if name == "x" else None) + assert cel.evaluate("x", ctx) == "resolved" + + def test_replacing_resolver_takes_effect(self): + ctx = Context() + ctx.set_variable_resolver(lambda name: 1) + assert cel.evaluate("anything", ctx) == 1 + ctx.set_variable_resolver(lambda name: 2) + assert cel.evaluate("anything", ctx) == 2 + + +class TestReuseAcrossCalls: + def test_same_context_serves_many_programs(self): + ctx = Context({"price": 10, "quantity": 5}) + ctx.add_function("discount", lambda total, rate: total * rate) + assert cel.compile("price * quantity").execute(ctx) == 50 + assert cel.compile("discount(price * quantity, 0.5)").execute(ctx) == 25.0 + assert cel.evaluate("quantity > 3", ctx) is True + + def test_many_executions_return_consistent_results(self): + ctx = Context({"items": list(range(100))}) + add_stdlib_to_context(ctx) + program = cel.compile("size(items.filter(i, i % 2 == 0)) + math.abs(-1)") + assert [program.execute(ctx) for _ in range(200)] == [51] * 200 + + def test_functions_and_resolver_together(self): + ctx = Context() + ctx.add_function("shout", lambda s: s.upper() + "!") + ctx.set_variable_resolver(lambda name: {"greeting": "hi"}.get(name)) + assert cel.evaluate("shout(greeting)", ctx) == "HI!" + + def test_dict_context_still_accepts_callables(self): + """Dict contexts are rebuilt per call, and keep sorting callables into functions.""" + program = cel.compile("twice(x)") + assert program.execute({"x": 2, "twice": lambda v: v * 2}) == 4 + assert program.execute({"x": 5, "twice": lambda v: v * 2}) == 10 + + +class TestReentrancy: + def test_callback_may_mutate_the_context_it_runs_under(self): + """A function registered on a context can modify that same context. + + The change is not visible to the evaluation already in progress (it runs + against a snapshot) but is visible to the next one. + """ + ctx = Context({"n": 1}) + + def bump(): + ctx.add_variable("n", 100) + return "bumped" + + ctx.add_function("bump", bump) + assert cel.evaluate("[bump(), string(n)]", ctx) == ["bumped", "1"] + assert cel.evaluate("n", ctx) == 100 + + def test_callback_may_evaluate_with_the_same_context(self): + ctx = Context({"base": 40}) + + def nested(): + return cel.evaluate("base + 1", ctx) + + ctx.add_function("nested", nested) + assert cel.evaluate("nested() + 1", ctx) == 42 + + def test_callback_may_execute_a_program_with_the_same_context(self): + ctx = Context({"depth": 0}) + inner = cel.compile("depth + 1") + + ctx.add_function("inner", lambda: inner.execute(ctx)) + assert cel.compile("inner() * 2").execute(ctx) == 2 + + +class TestThreads: + def test_shared_context_across_threads(self): + ctx = Context({"x": 3, "y": 4}) + ctx.add_function("hyp", lambda a, b: (a * a + b * b) ** 0.5) + program = cel.compile("hyp(x, y) + double(x)") + + def work(_): + return [program.execute(ctx) for _ in range(100)] + + with ThreadPoolExecutor(max_workers=8) as pool: + results = list(pool.map(work, range(8))) + + assert all(result == [8.0] * 100 for result in results) + + def test_mutation_from_one_thread_is_seen_by_others(self): + ctx = Context({"v": 1}) + assert cel.evaluate("v", ctx) == 1 + + def mutate(): + ctx.add_variable("v", 2) + + with ThreadPoolExecutor(max_workers=1) as pool: + pool.submit(mutate).result() + + assert cel.evaluate("v", ctx) == 2 From 9e98fdbe926ffc0cea865de567e761a0de008c05 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 02:12:50 +0000 Subject: [PATCH 2/2] Invalidate the cached environment before mutating, not after update() applies entries in order and returns early on the first bad key or value, so with the invalidation at the end a failed call left earlier entries applied but the cache untouched: evaluations kept returning the old value until some later successful mutator happened to drop the cache. Every mutator now drops it first. Nothing can repopulate the cache during the mutator because it holds &mut self. Tests pin both the failed-update and failed-add_variable cases. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW --- src/context.rs | 14 +++++++++++--- tests/test_context_reuse.py | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/context.rs b/src/context.rs index a5a59db..4a7943b 100644 --- a/src/context.rs +++ b/src/context.rs @@ -92,6 +92,12 @@ impl Context { } /// Drops the cached environment so the next evaluation rebuilds it. + /// + /// Every mutator calls this *before* touching `variables` or `functions`. + /// A mutator can fail part-way (`update()` rejects a later key after + /// inserting earlier ones), and invalidating up front means the cache can + /// never describe state the maps no longer hold. Nothing can repopulate it + /// while the mutator runs, because the mutator holds `&mut self`. fn invalidate(&mut self) { *self.cel.get_mut().unwrap_or_else(PoisonError::into_inner) = None; } @@ -254,8 +260,8 @@ impl Context { /// >>> context.add_function("regex_match", re.match) /// >>> # Note: This would need proper error handling in practice fn add_function(&mut self, name: String, function: Py) { - self.functions.insert(name, function); self.invalidate(); + self.functions.insert(name, function); } /// Registers a Python callable for lazy variable resolution. @@ -363,13 +369,13 @@ impl Context { /// >>> evaluate("counter", context) /// 2 pub fn add_variable(&mut self, name: String, value: &Bound<'_, PyAny>) -> PyResult<()> { + self.invalidate(); let value = crate::RustyPyType(value).try_into_value().map_err(|e| { pyo3::exceptions::PyValueError::new_err(format!( "Failed to convert variable '{name}': {e}" )) })?; self.variables.insert(name, value); - self.invalidate(); Ok(()) } @@ -460,6 +466,9 @@ impl Context { /// >>> evaluate('join(["user", name, string(age)])', context) /// 'user-Bob-30' pub fn update(&mut self, variables: &Bound<'_, PyDict>) -> PyResult<()> { + // Before the loop, not after: a bad key or value part-way through + // returns early with the earlier entries already applied. + self.invalidate(); for (key, value) in variables { // Attempt to extract the key as a String let key = key @@ -479,7 +488,6 @@ impl Context { self.variables.insert(key, value); } } - self.invalidate(); Ok(()) } diff --git a/tests/test_context_reuse.py b/tests/test_context_reuse.py index 411ecd7..08830de 100644 --- a/tests/test_context_reuse.py +++ b/tests/test_context_reuse.py @@ -53,6 +53,32 @@ def test_update_after_first_evaluation(self): ctx.update({"a": 10, "b": 5, "add": lambda x, y: x + y}) assert cel.evaluate("add(a, b)", ctx) == 15 + def test_failed_update_does_not_leave_a_stale_cache(self): + """A mutator that raises part-way must still drop the cached environment. + + ``update()`` applies entries in order and raises on the first bad one, so + ``x`` is already ``2`` when the unconvertible value is hit (pre-existing + behaviour). Evaluation must then agree with the context's state rather + than keep serving the snapshot taken before the failed call. + """ + ctx = Context({"x": 1}) + assert cel.evaluate("x", ctx) == 1 + + with pytest.raises(ValueError): + ctx.update({"x": 2, "bad": object()}) + + assert cel.evaluate("x", ctx) == 2 + + def test_failed_add_variable_does_not_leave_a_stale_cache(self): + ctx = Context({"x": 1}) + assert cel.evaluate("x", ctx) == 1 + with pytest.raises(ValueError): + ctx.add_variable("bad", object()) + # Nothing changed, and the next evaluation is still correct. + assert cel.evaluate("x", ctx) == 1 + ctx.add_variable("x", 3) + assert cel.evaluate("x", ctx) == 3 + def test_resolver_set_after_first_evaluation(self): ctx = Context({"static_var": 1}) assert cel.evaluate("static_var", ctx) == 1