Repository navigation
sc-12787: wire up CI on GitHub-hosted macos-15 (validate.yml never executed) + fix everything it catches - #17
Merged
Conversation
None of these are new breakage — they are what the first-ever CI run would have flagged on current main: - mlx-rs/src/memory.rs: cargo fmt --check fails on main (rustfmt wants a trailing-comment continuation alignment); restructure with a blank line instead of accepting rustfmt's column-55 comment. - examples/mnist/src/main.rs: new stable rustc future-compat lint float-literal-f32-fallback fails clippy -D warnings; pin 1e-2_f32. - mlx-rs/Cargo.toml: set [lib] name = "mlx_rs". The fork renamed the package to pmetal-mlx-rs but everything still expects the upstream crate name: workspace dependents rename it back, the internal proc macros emit ::mlx_rs paths, and doctests + the crate's own examples link by real lib name — all 214 doctests and both mlx-rs examples were uncompilable since the rename. - mlx-sys/examples/is_metal_available.rs: same rename rot; reference pmetal_mlx_sys directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MLX 0.32 made default streams thread-local and evaluates lazy graph nodes on the calling thread; evaluating a node recorded on another thread's stream throws 'There is no Stream(gpu, N) in current thread' (metal::get_command_encoder). random::GLOBAL_STATE is the one array shared across threads, and its key was advanced lazily — so a key recorded on thread A crashed the next thread to consume and evaluate it. Reproduced by 10/12 tests in mlx-tests test_compile_with_state (libtest runs each test on a fresh thread) and by any multithreaded consumer of the global-key fallback. Evaluate the global state array at every write point (init, resolve_global_key, seed) before the lock is released; evaluation always happens on the thread that recorded the ops, so it is safe. Task-local and user-owned RandomState keep their lazy semantics. Regression test test_global_rng_across_threads drives the exact cross-thread sequence; mutation-checked (reverting the resolve_global_key hunk reproduces the production error). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Upstream oxideai/mlx-rs runs CI on paid blaze/* runners the SceneWorks fork has no subscription to, so validate.yml and publish-docs.yml have had zero workflow runs ever and fork PRs got no checks. validate.yml: run checks and tests on GitHub-hosted macos-15 (free for this public fork; arm64; image ships Xcode 16.4 — the pinned version and image default — plus the xcodes CLI, so existing steps work unchanged). Add timeout-minutes guards (cold MLX C++ build on a 3-core VM; default 6h would eat the 5-job macOS concurrency pool if wedged), least-privilege contents:read, and reopened/ready_for_review PR triggers. publish-docs.yml: same runner retarget, but manual-dispatch only — GitHub Pages is not enabled on the fork, so an auto-run on every merge to main would fail at Setup Pages. Enabling Pages and restoring the push trigger is tracked in sc-12933. Deliberately NOT the org's self-hosted runners: on a public repo, fork PRs would run arbitrary code on that hardware. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
First CI run result: checks (clippy + full MLX build) PASSED on GitHub-hosted macos-15 in 4m30s, but both tests legs failed before compiling anything: with no committed lockfile, CI resolves deps fresh, and icu_*/idna_adapter releases now require rustc 1.86 — the resolved tree had drifted past rust-version = 1.85.0. Resolver "3" (cargo 1.84+) makes resolution respect rust-version, holding those back to MSRV-compatible versions (icu 2.1.1, idna_adapter 1.2.1). Verified locally: cargo +1.85.0 test --all --no-run builds the entire workspace and every test binary; full stable gate (fmt, clippy -D warnings, cargo test --all) still green under the new resolution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
michaeltrefry
added a commit
that referenced
this pull request
Jul 18, 2026
This branch's merge ref carried the old validate.yml targeting paid blaze/* runner labels the fork has no subscription to, so PR #18's jobs sat QUEUED forever. Take the validate.yml from sc-12787 (PR #17, GitHub-hosted macos-15, proven green there) byte-identically — whichever PR merges first, the other merges this file cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- random.rs: restructure the global-state write points so the state is only ever REPLACED BY an already-evaluated array. Previously resolve_global_key/seed mutated the state first and evaluated after, so an eval error released the lock with a lazy state and reopened the cross-thread crash window. seed() now builds and evaluates the new key before taking the lock. Also drops the imprecise sibling-outputs comment (split's outputs are separate slice ops). - Cargo.toml (all members): inherit rust-version.workspace = true. MSRV-aware resolution keys off member rust-version fields; only examples/lm inherited it, so the 1.85 leg's protection depended on the icu/idna chain being reachable from that one example. Now every member carries MSRV metadata (also fixes published-crate metadata). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
michaeltrefry
added a commit
that referenced
this pull request
Jul 18, 2026
…olves The 1.85.0 job failed: with no committed lockfile CI resolves fresh, and new transitive releases (icu_* 2.2, idna_adapter 1.2.2) now require rustc 1.86. Adopt sc-12787's fix byte-identically — resolver = "3" (MSRV-aware, respects workspace rust-version = 1.85.0) plus rust-version.workspace = true in every member crate — along with its memory.rs fmt fix and mlx-rs/Cargo.toml [lib] block, so those files merge cleanly whichever of #17/#18 lands first. Two deliberate divergences from #17, kept from sc-12940's design and now guarded by a comment in is_metal_available.rs: mlx-sys keeps [lib] name = "mlx_sys" and its example keeps upstream `mlx_sys::` paths (#17 instead renamed the example to pmetal_mlx_sys::, which stops resolving once the [lib] rename is in place — whoever merges second must keep the [lib]-name form). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
michaeltrefry
added a commit
that referenced
this pull request
Jul 18, 2026
The merge of main (#17) auto-applied its pmetal_mlx_sys:: rename to the example without a textual conflict, but that name no longer resolves under this branch's [lib] name = "mlx_sys". Keep the upstream-named path, matching the [lib]-rename treatment #17 itself chose for mlx-rs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 18, 2026
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.
Shortcut sc-12787
Why
Both workflows target
blaze/*runner labels — a paid third-party macOS runner service upstream oxideai/mlx-rs subscribes to but this fork does not. Jobs are never picked up: zero workflow runs in the repo's history, so PRs #12–#16 merged with no CI. This PR's own checks are the first CI execution ever.Runner decision
macos-15: free for this public fork, arm64, image ships Xcode 16.4 (the pinned version, also the image default) and thexcodesCLI, so the existing workflow steps run unchanged. Matches the inference repo's macOS PR lane.Changes
.github/workflows/validate.yml— retargetblaze/macos-15→macos-15; addtimeout-minutes(90 checks / 180 tests: cold MLX C++ builds on a 3-core VM, and a wedged Metal job would otherwise hold a 6h slot in the 5-job macOS concurrency pool), top-levelpermissions: contents: read, andreopened/ready_for_reviewPR trigger types..github/workflows/publish-docs.yml— retarget tomacos-15, manual-dispatch only: GitHub Pages is not enabled on this fork, so an auto-run on every merge to main would fail at Setup Pages. Enabling Pages (publishes the fork's rustdoc) is a product decision tracked in sc-12933.Pre-existing breakage the new CI immediately caught (all fixed here)
Running validate.yml's exact commands against current main fails in four independent ways — none of them introduced by this PR:
mlx-rs/src/random.rs). MLX 0.32 made default streams thread-local and evaluates lazy nodes on the calling thread. The process-globalrandom::GLOBAL_STATEkey was advanced lazily, so a key recorded on thread A's stream crashed the next thread to evaluate it:There is no Stream(gpu, 0) in current thread. 10/12 tests inmlx-tests --test test_compile_with_statefailed (libtest runs each test on its own thread); any multithreaded consumer of the global-key fallback can hit it. Fix: eagerly evaluate the global state array at each write point before releasing the lock (task-local/user-ownedRandomStatekeeps lazy semantics). Regression testtest_global_rng_across_threadsadded and mutation-checked — reverting the fix reproduces the exact production error.pmetal-mlx-rsbut doctests and a crate's own examples link the lib by its real name, while dependents, doc examples, and the internal proc macros (which emit::mlx_rspaths) all expectmlx_rs. Fix:[lib] name = "mlx_rs"(dependents'mlx-rs = { package = "pmetal-mlx-rs" }rename is unaffected). Same rot fixed inmlx-sys/examples/is_metal_available.rs.cargo fmt --checkfails on main — one comment-alignment hunk inmlx-rs/src/memory.rs.clippy -D warningsfails on latest stable — newfloat-literal-f32-fallbackfuture-compat lint inexamples/mnist/src/main.rs(1e-2→1e-2_f32).Validation
Local full gate mirroring CI, all green:
cargo fmt -- --check·cargo clippy -- -D warnings·cargo test --all -- --test-threads=1(590 lib tests, all integration suites, 214 doctests). One genuinely CI-only unknown remains: MLX Metal kernel behavior on GitHub's paravirtualized VM GPU — this PR's run is the deciding experiment, which is precisely the story's point.🤖 Generated with Claude Code