fix(setup): install mlx-lm and mlx-audio in setup-python on Apple Silicon#892
fix(setup): install mlx-lm and mlx-audio in setup-python on Apple Silicon#892flowbrix wants to merge 2 commits into
Conversation
…icon The dev setup installed requirements-mlx.txt but not mlx-audio/mlx-lm themselves, so POST /transcribe failed on a fresh Apple Silicon setup with "No module named 'mlx_audio'" (then "No module named 'mlx_lm'"). The release workflow already installs both with --no-deps (they declare transformers>=5.x, conflicting with our <=4.57.x cap); mirror that in the setup-python recipe with the same pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Apple Silicon ChangesMLX setup and validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
justfile (1)
75-80: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd an automated MLX smoke test. The
--no-depssplit is already documented, butbackend/tests/test_qwen_download.pyis a manual script and won’t run underpytest, so themlx_audioimport/download path still isn’t guarded in CI. Add a small smoke check formlx_audioplus one TTS/STT path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@justfile` around lines 75 - 80, The MLX installation commands lack an automated CI smoke test for the mlx_audio import and runtime path. Add a pytest-discoverable smoke test near the existing Qwen download test that imports mlx_audio and exercises one TTS or STT operation, keeping the check small and compatible with the installed MLX dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@justfile`:
- Around line 75-80: The MLX installation commands lack an automated CI smoke
test for the mlx_audio import and runtime path. Add a pytest-discoverable smoke
test near the existing Qwen download test that imports mlx_audio and exercises
one TTS or STT operation, keeping the check small and compatible with the
installed MLX dependencies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e17eb7c-4edc-492c-acc1-752d3e2cbbf6
📒 Files selected for processing (2)
backend/requirements-mlx.txtjustfile
mlx-audio and mlx-lm are installed --no-deps, so a missing transitive dependency only surfaces at import time. Add a pytest-discoverable smoke test (skipped off Apple Silicon) covering the exact entry points the backend uses: mlx_audio.tts.load, mlx_audio.stt.load (which also exercises the miniaudio dep from issue jamiepine#505), mlx_lm.load/generate, and a basic mlx.core op. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the CodeRabbit nitpick: added 🤖 Addressed by Claude Code |
Problem
On a fresh Apple Silicon dev setup,
just setupinstallsbackend/requirements-mlx.txtbut notmlx-audio/mlx-lmthemselves, so the Whisper MLX STT path is broken out of the box:POST /transcribefails withNo module named 'mlx_audio'(and, once that is installed,No module named 'mlx_lm').These two packages are intentionally absent from
requirements-mlx.txtbecause frommlx-audio0.3.1 /mlx-lm0.31.1 onward they declaretransformers>=5.x, which conflicts with thetransformers<=4.57.xcap inrequirements.txt(see the existing note inbackend/requirements-mlx.txt). The release workflow (.github/workflows/release.yml) already handles this by installing both with--no-deps— but the dev setup recipe never got the same treatment.Fix
Mirror the release workflow in the Apple Silicon branch of the
setup-pythonrecipe ([unix]section;[windows]is unaffected since MLX is macOS-only):pip install --no-deps mlx-lm==0.31.1pip install --no-deps mlx-audio==0.4.1with a comment explaining the
--no-deps(transformers conflict), plus a pointer to the justfile in therequirements-mlx.txtnote. Pins matchrelease.ymlso dev and CI stay in sync.Verification
Ran
just setup-pythonin a clean checkout (no pre-existing venv) on an Apple Silicon Mac — completed successfully, and:with
mlx-audio 0.4.1/mlx-lm 0.31.1resolved as pinned.🤖 Generated with Claude Code
Summary by CodeRabbit