Open
Conversation
Node.js 20 is deprecated on GitHub runners (forced Node 24 default by June 2026). Bumps the following actions across release, benchmark and trufflehog workflows: - actions/checkout v4 -> v6.0.2 - actions/setup-python v5 -> v6.2.0 - actions/upload-artifact v4 -> v7.0.1 - actions/download-artifact v4 -> v8.0.1 - actions/cache v4 -> v5.0.5 - PyO3/maturin-action v1 -> v1.51.0 (pinned SHA) - dtolnay/rust-toolchain @stable -> pinned SHA All pins follow the repo convention: commit SHA + version comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- swap goto-bus-stop/setup-zig (unmaintained) for mlugg/setup-zig; the former was failing `zig env` on current Ubuntu runners, which napi-rs calls from patchArmFeaturesHForArmTargets for ARM/musl/freebsd cross-compiles. - bump rust inside the x86_64-unknown-linux-gnu docker build: the pinned napi-rs lts-debian image ships Rust 1.82.0, but transitive deps now require edition2024 (Rust 1.85+). `rustup update stable` pulls a current toolchain before invoking `yarn build`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
napi-rs no longer supports FreeBSD under --zig, and cross-compilation via a FreeBSD VM is a separate, slower setup. Drop the target from the build matrix and the napi triples list; FreeBSD users can build from source if needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
napi-rs's patchArmFeaturesHForArmTargets fails to find zig on PATH under both goto-bus-stop/setup-zig and mlugg/setup-zig on current runners. Skip the target for now to unblock the release; can be revisited with a native ARM runner or a pinned zig version later. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
actions/setup-python is pulling a broken python-3.14.4-win32-arm64- freethreaded package (0-byte python.exe, pip install fails with ModuleNotFoundError: encodings). Upstream breakage in actions/python- versions. Keep the regular 3.14 arm64 build; free-threaded arm64 Windows wheels are extremely niche and can be added back when the upstream package is fixed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@napi-rs/cli removed the --name option; the name is now read from the napi.name field in package.json. Replace with --dir ./artifacts so the command picks up the downloaded per-target .node files. Also pins the second PyO3/maturin-action reference (inside the build job) that was missed by the earlier action-modernization commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Without this attribute, PyO3 emits a module-init path that aborts under free-threaded Python (Py_MOD_GIL_USED default), causing SystemError: init function of tokenizers returned uninitialized object on 3.14t. Tokenizer state is largely immutable post-construction and the BPE cache was recently made per-thread (no shared mutable state), so opting into GIL-free execution is safe here. Applied to all top-level #[pymodule] declarations: tokenizers, decoders, models, normalizers, pre_tokenizers, processors, trainers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
napi-rs/cli's `napi universal` subcommand looks up the universal arch entry in the napi.triples config. Without `universal-apple-darwin` in the additional list the command errors with 'universal' arch for platform 'darwin' not found in config! Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
maturin-action v1.51 auto-enables sccache, but inside the manylinux docker containers sccache can't reach the GHA cache service and times out during rustc version detection: sccache: error: Timed out waiting for server startup 💥 maturin failed Disabling sccache restores the pre-v1.51 behaviour. Compilation is slower but correct; if we later want caching for the non-container builds we can switch on/off conditionally via the matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Silences the long-standing `unexpected_cfg(feature = "noop")` warning from the napi attribute macro (only fixed in the v3 generator) and brings us onto a maintained major version. Changes: - Cargo.toml: napi/napi-derive "2" -> "3" (also pulls napi-sys 3, napi-derive-backend 5) - package.json: @napi-rs/cli "^2.14.6" -> "^3.6.2"; napi.name/triples replaced with napi.binaryName/targets (v3 format, `defaults: true` is gone — targets must be listed explicitly); drop `--pipe "prettier -w"` from build scripts because v3 pipes the .node binary through the formatter too - src/encoding.rs, src/pre_tokenizers.rs: return Vec<u32> / nested Vecs instead of manually constructed `Array`s (the v3 `Array<'env>` lifetime prevents returning from the function). `ts_return_type` attrs preserve the public TS tuple types. - index.js / index.d.ts: regenerated by the v3 CLI Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The real cause of `SystemError: init function of tokenizers returned uninitialized object` on Python 3.14t was abi3: the extension was built with the `abi3`/`abi3-py310` pyo3 features hardcoded in Cargo.toml, which maturin can't strip when it detects free-threaded Python. The resulting `tokenizers.abi3.so` can't be imported by 3.14t because free-threaded Python doesn't expose the stable/limited API. Changes: - Cargo.toml: promote `abi3` to a project-level cargo feature (default on). Maturin auto-drops it for free-threaded interpreters now that it's project-owned rather than a pyo3 dep-feature. - pyproject.toml: add `abi3` to `[tool.maturin].features`. - python.yml: install via `maturin develop` instead of `pip install -e .[dev]`. pip's PEP 660 editable path bypasses maturin's free-thread detection and writes `.abi3.so` anyway. - Revert the earlier `gil_used = false` attempt: it didn't fix the error (abi3 was the real cause) and was semantically wrong — tokenizer components are mutable via `tokenizer.post_processor.x = y` etc., so claiming GIL-free safety would invite races. - Also bump pyo3 0.28.2 -> 0.28.3. Verified locally on 3.14t: 191 tests pass, 2 skipped. The 2 remaining failures in tests/documentation are unrelated (truncated tokenizer-wiki.json data fixture). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
maturin detects the abi3/3.14t mismatch and retags the wheel as cp314-cp314t, but it does NOT drop the abi3 cargo feature when invoking the compiler — so the resulting .so is still abi3-built and can't load under free-threading. Gate the feature via sys._is_gil_enabled() and pass --no-default-features --features ext-module when the GIL is disabled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`ty check` (used by `make check-style`) rebuilds its own cdylib with the package's abi3 feature enabled and then imports it for type introspection. Under 3.14t that import fails with: SystemError: init function of tokenizers returned uninitialized object The same style check runs on regular 3.14 in the matrix, so skipping it on 3.14t doesn't lose coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lint (rustfmt, clippy), cargo-audit, and python style checks produce the same result regardless of OS / Python version, so running them on every matrix cell is wasted work. - rust.yml: gate RustFmt / benchmarks RustFmt / Clippy / cargo-audit install + run to ubuntu-latest - python.yml: gate RustFmt / Clippy / cargo-audit / `make check-style` to ubuntu-latest + Python 3.14 Build and test steps stay on the full matrix because those do vary across OS / interpreter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
actions-rs/toolchain and actions-rs/cargo run on Node 20 and are unmaintained — they emit deprecation warnings on current runners. - Replace actions-rs/toolchain@v1 with dtolnay/rust-toolchain@stable (already used by node.yml). For the windows-32 build, switch from `toolchain: stable-i686-pc-windows-msvc` host install to the x86_64 host with `targets: i686-pc-windows-msvc`, and pass `--target` to cargo build explicitly. Also drop the now-redundant `Override toolchain` step that rewrote ./bindings/python/rust-toolchain. - Replace every actions-rs/cargo wrapper with plain `run: cargo ...` — the wrapper was only adding an extra action hop. - Bump actions/setup-python pin v5 -> v6.2.0 in python.yml (the build_win_32 + build_and_test jobs were missed by the earlier modernization sweep). Also split bindings/python/Makefile into `test-py` / `test-rs` / `test` targets. The cargo side uses pyo3 `auto-initialize` which links libpython; free-threaded Python 3.14t on macOS runners doesn't ship libpython3.14t in the framework path, so CI now runs `make test-py` only on 3.14t and full `make test` on regular interpreters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Node and Python bindings/release tooling (notably moving the Node bindings to the NAPI-RS v3 toolchain and enabling abi3 for the Python extension) and refreshes CI workflows/actions pins.
Changes:
- Node bindings: migrate to
@napi-rs/cliv3 +napi/napi-derivev3, regenerate loader (index.js) and TypeScript definitions, and update release workflow packaging steps. - Python bindings: bump
pyo3, add anabi3feature and enable it by default, and split Makefile test targets to avoid known free-threaded Python runner issues. - CI: replace deprecated
actions-rs/*usage with directcargocommands /dtolnay/rust-toolchain, and pin multiple GitHub Actions by commit SHA.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| bindings/python/pyproject.toml | Enable abi3 in maturin feature set. |
| bindings/python/Makefile | Split tests into test-py and test-rs; test runs both. |
| bindings/python/Cargo.toml | Bump pyo3 and introduce abi3 feature (enabled by default). |
| bindings/node/yarn.lock | Lockfile changes from upgrading Node dev tooling (incl. @napi-rs/cli). |
| bindings/node/src/pre_tokenizers.rs | Change NAPI API surface for pre-tokenization return values. |
| bindings/node/src/encoding.rs | Change NAPI API surface for offset-related return values. |
| bindings/node/package.json | Switch NAPI-RS config to v3 schema; update build scripts and CLI version. |
| bindings/node/index.js | Regenerated native-binding loader for NAPI-RS v3. |
| bindings/node/index.d.ts | Regenerated TypeScript declarations. |
| bindings/node/Cargo.toml | Bump napi and napi-derive to v3. |
| .github/workflows/trufflehog.yml | Pin actions/checkout by SHA. |
| .github/workflows/rust.yml | Switch to dtolnay/rust-toolchain and direct cargo commands; reduce OS-invariant linting duplication. |
| .github/workflows/rust-release.yml | Pin actions; update cache action version. |
| .github/workflows/python.yml | Update toolchain + maturin install/build logic for 3.14/3.14t; adjust test invocation. |
| .github/workflows/python-release.yml | Pin actions; adjust some matrix entries (but still builds 3.14t broadly). |
| .github/workflows/node-release.yml | Update zig action; adjust build matrix and universal packaging command. |
| .github/workflows/benchmarks.yml | Pin actions/upload-artifact by SHA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two changes, both cleaning up drift from earlier platform-matrix cuts: 1. python-release.yml: remove `3.14t` from every matrix entry and from the default --interpreter. bindings/python defaults the abi3 cargo feature on, and maturin only retags the wheel — it doesn't drop the feature for free-threaded builds, so 3.14t wheels would compile with abi3 and fail to load under free-threaded Python. CI continues to test 3.14t on the PR path; we just don't ship release wheels for it until we audit the extension for thread safety. 2. bindings/node/npm/: delete the stale `freebsd-x64` and `linux-arm-gnueabihf` per-target npm package templates. Those targets were removed from the release matrix and `napi.targets` earlier; the templates would otherwise drive `yarn artifacts` at publish time to either create empty packages or fail when the `.node` artifact isn't present. Note on the generated bindings/node/index.js: napi-cli v3 hardcodes loader branches for every platform regardless of `napi.targets`. The freebsd/armv7-gnueabihf branches remain as dead code in index.js; on those platforms the `require()` calls fail-through to an aggregate "no prebuilt binary" error, which is the intended behaviour. Users on dropped platforms need to build from source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the advertised TS tuple types. Previously returned Vec<u32> with `ts_return_type = "[number, number] | ..."`, which compiled but at runtime produced `number[]` of variable length — so the public TS contract could be violated if the Rust side ever emitted a different length. - encoding.rs word_to_tokens / word_to_chars / token_to_chars: Vec<u32> -> (u32, u32). napi 3 serializes Rust tuples to JS arrays, so the ABI is identical and the length is now enforced at compile time. - pre_tokenizers.rs pre_tokenize_string: inner Vec<u32> -> (u32, u32) for the offset pair inside each entry. Regenerated index.js / index.d.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds seven matrix cells that build wheels for free-threaded Python 3.14t on mainstream platforms (linux x86_64/aarch64 manylinux + musllinux, macos x86_64/aarch64, windows x86_64). The 3.14t cells pass `--no-default-features --features pyo3/extension-module` to maturin so the abi3 cargo feature is dropped — free-threaded Python can't load stable-ABI extensions. Discriminator: `flavor: [abi3]` is added to the base matrix so the 3.14t include: entries (with `flavor: ft`) become new cells instead of merging with the matching abi3 combos. Artifact names are suffixed with `-abi3` / `-ft` so the two wheels for the same os/target/manylinux don't collide on upload. Skipped for 3.14t: i686, armv7, ppc64le, s390x, windows-aarch64 — niche targets where 3.14t demand is negligible. They can be added later by copying the pattern. Note on runtime behaviour: `gil_used` stays at the default (true), so free-threaded Python will re-enable the GIL when tokenizers is imported (with a warning). Users who really need GIL-free can set `PYTHON_GIL=0`, but tokenizer components are mutable via `tokenizer.post_processor.x = y` etc., so races are possible until a thread-safety audit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to 21cbdbe: complete the 3.14t coverage instead of hand-waving "niche". Adds 6 more entries (linux i686, armv7, ppc64le, s390x; windows i686; windows-11-arm aarch64) bringing 3.14t to parity with the abi3 matrix. If any of these targets doesn't have a 3.14t interpreter available (plausible for 32-bit and for non-Tier-1 archs under setup-python / maturin-action's docker images) the release run will surface that loudly and we drop the row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously dropped because `mlugg/setup-zig` without a `version:` input installs zig master (nightly). napi-rs CLI v2 invokes `zig env` and parses its JSON output directly in `patchArmFeaturesHForArmTargets` — zig master has drifted enough that parsing fails with the opaque "Cannot get zig env correctly" error. fix-node-release is on CLI v3 (which uses `cargo-zigbuild` instead of the manual spawn), so the same zig-master issue no longer blocks us, but pinning is still the right call — 0.13.0 is the version napi-rs's own release templates use. - Re-add the target to the matrix and to napi.targets. - Pin mlugg/setup-zig to version 0.13.0. - Add a small `zig version && which zig` diagnostic so future breakage surfaces clearly rather than hiding inside napi-rs errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
No description provided.