Skip to content

Package the native runtime and default to Rust - #390

Open
nicosuave wants to merge 15 commits into
mainfrom
cutover-inventory
Open

nicosuave wants to merge 15 commits into
mainfrom
cutover-inventory

Conversation

@nicosuave

@nicosuave nicosuave commented Sep 14, 2026

Copy link
Copy Markdown
Member

Normal native installs now include a matching lightweight sidemantic-rs package and default to Rust for validation, compilation, and semantic SQL rewriting. Explicit Python and controlled auto fallback remain 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 full existing pytest suite separately with Python and Rust defaults; the Rust run does not enable fallback and failures fail the job. Explicit engine-specific contract tests retain their overrides.

This remains draft until the full Rust run and native package checks pass. The earlier passing reference tests do not establish Rust parity.

@nicosuave
nicosuave marked this pull request as ready for review September 14, 2026 18:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T18:43:46.060116Z e369b00 Draft marked ready
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +143 to +144
if engine is None and not any(key.startswith("SIDEMANTIC_RS_") for key in os.environ):
engine = default_engine()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Let SIDEMANTIC_ENGINE override legacy flags

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 👍 / 👎.

Comment thread pyproject.toml
license = {file = "LICENSE"}
requires-python = ">=3.11"
dependencies = [
"sidemantic-rs==0.12.0; sys_platform != 'emscripten'",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required Pyodide annotation to the native pin

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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant