Repository navigation
Package the native runtime and expand Rust parity - #390
Conversation
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e369b00738
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if engine is None and not any(key.startswith("SIDEMANTIC_RS_") for key in os.environ): | ||
| engine = default_engine() |
There was a problem hiding this comment.
When a Python process sets SIDEMANTIC_ENGINE=python but still has any SIDEMANTIC_RS_* variable—even one set to 0—this guard skips default_engine() and the legacy branch controls dispatch instead. For example, a leftover SIDEMANTIC_RS_SQL_GENERATOR=1 still enables Rust, so users migrating to the documented process-wide override cannot reliably force Python without first removing every legacy variable; check SIDEMANTIC_ENGINE before preserving legacy selection.
Useful? React with 👍 / 👎.
| license = {file = "LICENSE"} | ||
| requires-python = ">=3.11" | ||
| dependencies = [ | ||
| "sidemantic-rs==0.12.0; sys_platform != 'emscripten'", |
There was a problem hiding this comment.
This adds a version-constrained core dependency whose Emscripten exclusion is essential for keeping the package installable in Pyodide, but omits the required PYODIDE: comment. Without that repository-required annotation, routine dependency maintenance can remove or alter the compatibility marker without recognizing why it is necessary; annotate this pin explicitly.
AGENTS.md reference: AGENTS.md:L109-L113
Useful? React with 👍 / 👎.
6d15bd9 to
7ea0836
Compare
nicosuave
changed the title
1 participant
Normal native installs include the matching lightweight sidemantic-rs package while Python remains the default for validation, compilation, and semantic SQL rewriting. Rust is selectable through CLI/API engine options, project configuration, or SIDEMANTIC_ENGINE; the process-wide choice now takes precedence over legacy SIDEMANTIC_RS_* flags. Explicit Rust rejects unavailable or unsupported capabilities, and controlled auto fallback remains available. Pyodide excludes the native dependency and keeps Python.
The release workflow versions and publishes the native package before the parent package, with Linux, macOS, and Windows wheels. CI runs the shared pytest suite separately with Python and Rust selected; the Rust run does not enable fallback. Existing planner, semantic handoff, validation, and parity improvements remain included. Base-install smoke coverage checks both the Python default and explicit Rust selection.
Query-scoped capability handling and native graph reuse are deferred prerequisites for a later Rust-default change. This PR stages packaging and parity without imposing the current whole-graph capability rejection or compilation cost on ordinary queries.
Focused validation passed with 220 tests under Python selection and 310 under Rust selection, plus installed-wheel API and CLI checks for both engines. Regression coverage includes environment precedence and the supported TMDL query that previously failed under the Rust default.