Fix import cel failing in a clean install; smoke test the wheel in CI - #44
Merged
Merged
Conversation
cel.cli imported Annotated from typing_extensions, which is not a declared dependency of this package. It used to arrive transitively through Typer, but Typer 0.21.2 (February 2026) dropped that dependency, so installing the wheel into an otherwise empty environment left `import cel` raising ModuleNotFoundError. The development lockfile masked the problem because mypy still pulls typing_extensions in. Annotated has been in the standard library since Python 3.9 and this package requires 3.11, so import it from typing. The lint job now builds the wheel, installs it into an empty virtual environment and runs both the import and the `cel` command, so an undeclared runtime dependency fails CI instead of the first user install. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This was referenced Sep 14, 2026
Resolves the CHANGELOG Unreleased conflict with #52 by folding the import fix into the shared Fixed section. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
The wheel is not abi3, so uv venv --no-project must not pick whichever Python the runner discovers first.
hardbyte
added a commit
that referenced
this pull request
Sep 15, 2026
## Why Every `evaluate()` and `Program.execute()` call rebuilt the cel-rust context from scratch: each variable re-boxed into a `Box<dyn Val>`, each registered Python function re-wrapped in a fresh closure. So the per-call cost scaled with the size of the Python `Context`, not the expression. The CLI, which registers all 47 extended-stdlib functions, paid ~7.5 µs of setup to evaluate `1 + 2`. ## What `Context` now builds its cel environment on first use and caches it (`Mutex<Option<Arc<cel::Context<'static>>>>`). `add_variable`, `add_function` and `update` drop the cache. `set_variable_resolver` does not need to: the resolver is bound per call in a child scope via cel-rust's `Context::new_inner_scope`, which keeps exactly the lookup order the resolver had on the root (resolver → registered variables → type identifiers), and leaves the root untouched and shareable. The evaluation borrows an `Arc` clone rather than the Python object, so a callback may mutate, or re-enter evaluation with, the `Context` it was registered on; the mutation applies from the next evaluation, which is also what happened before (the old code copied variables and functions out before executing). Dict contexts are still materialised per call, because a dict can change between calls without notice. Also in this PR, because the same code was being rewritten: the Python-function wrapper and the compile step are factored into helpers shared by `evaluate()`/`compile()`, and the panic message for an execution-time panic no longer calls itself a parser error. ## Numbers `min` of 3 × 50 000 iterations, release build, same machine. "Before" is `main`'s Rust (built from the #44 branch, which only touches Python). | `Program.execute()` against… | before | after | |---|---:|---:| | no context | 0.13 µs | 0.15 µs | | dict, 2 vars | 0.43 µs | 0.54 µs | | `Context`, 2 vars | 0.31 µs | 0.18 µs | | `Context`, 2 vars + 1 Python fn (called) | 0.70 µs | 0.37 µs | | `Context`, 2 vars + resolver | 0.43 µs | 0.24 µs | | `Context` with the 47 extended-stdlib functions | **7.48 µs** | **0.16 µs** | | `Context` with 200 vars | **31.85 µs** | **0.13 µs** | The no-context and dict rows are within run-to-run noise (an alternating A/B of 7 × 100 000 iterations gave 0.13–0.14 vs 0.13–0.14 and 0.45–0.48 vs 0.42–0.48). ## Tests New `tests/test_context_reuse.py` pins the contract: every mutator is visible on the next evaluation (including replacing a function and replacing the resolver); resolver precedence over registered variables is unchanged; a callback can mutate the context it runs under and can call `evaluate()`/`execute()` with that same context; one `Context` serves many programs; a `Context` shared across 8 threads returns correct results. 528 passed, 1 skipped, 5 xfailed. `cargo fmt --check`, `cargo clippy -D warnings`, `cargo test`, ruff and mypy clean. Note for merging: this and #44 both add to the CHANGELOG's Unreleased section, so whichever lands second will need a trivial conflict resolution there.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
pip install common-expression-languageinto an otherwise empty environment currently gives:cel.cliimportsAnnotatedfromtyping_extensions, which is not a declared dependency. It used to arrive transitively through Typer, but Typer 0.21.2 (2026-02-10) dropped that dependency, so every release from 0.6.0 onward is affected for users who don't happen to havetyping_extensionsinstalled for another reason. The development lockfile hides this because mypy still pulls it in, which is why the test suite never noticed.Reproduced by building the wheel and installing it into a fresh
uv venv --no-project:import celfails onmain, and passes with this change. The clean environment ends up with exactlyannotated-doc, prompt-toolkit, pygments, rich, shellingham, typer, markdown-it-py, mdurl, wcwidthplus the wheel.What
from typing import Annotated— in the standard library since 3.9, and this package requires 3.11.import cel,cel.evaluate, and thecelcommand. That's the only kind of environment where an undeclared runtime import shows up, so it becomes a CI failure rather than a first-install failure.This is worth a 0.9.1 patch release on its own; I've left the version bump for the usual "Prepare release" commit.
Verification
Locally: pytest (511 passed, 1 skipped, 5 xfailed),
ruff check,ruff format --check, mypy, and the wheel smoke test above in a clean venv.https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
Generated by Claude Code