Skip to content

tokenizer: return successful mlx-lm load result#215

Open
krystophny wants to merge 2 commits intowaybarrios:mainfrom
computor-org:feature/fix-tokenizer-load-return
Open

tokenizer: return successful mlx-lm load result#215
krystophny wants to merge 2 commits intowaybarrios:mainfrom
computor-org:feature/fix-tokenizer-load-return

Conversation

@krystophny
Copy link
Copy Markdown
Contributor

@krystophny krystophny commented Mar 24, 2026

Summary

  • fix load_model_with_fallback() so a successful mlx_lm.load() returns (model, tokenizer) instead of falling through as None
  • add a regression test for the successful return path

Main files

  • vllm_mlx/utils/tokenizer.py
  • tests/test_tokenizer_utils.py

Reviewer focus

The bug is small but user-visible: a valid MLX load can succeed internally and still surface as a failed startup because the successful tuple is not returned.

Tests

  • PYTHONPATH=/Users/ert/code/vllm-mlx /Users/ert/code/.venv/bin/python -m pytest tests/test_tokenizer_utils.py -q
  • python3 -m compileall vllm_mlx

@krystophny krystophny changed the title Fix successful MLX tokenizer loads tokenizer: return successful mlx-lm load result Mar 24, 2026
Copy link
Copy Markdown
Collaborator

@Thump604 Thump604 left a comment

Choose a reason for hiding this comment

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

Good catch — the mlx-lm fallback path was silently returning None. The regression test is a nice touch. +1

krystophny added a commit to computor-org/vllm-mlx that referenced this pull request Mar 25, 2026
Add test confirming deduplicated terminal blocks correctly isolate
recurrent state per sequence. Remove the duplicate tokenizer return fix
that already ships in PR waybarrios#215.
krystophny added a commit to computor-org/vllm-mlx that referenced this pull request Mar 26, 2026
Add test confirming deduplicated terminal blocks correctly isolate
recurrent state per sequence. Remove the duplicate tokenizer return fix
that already ships in PR waybarrios#215.
krystophny added a commit to computor-org/vllm-mlx that referenced this pull request Mar 26, 2026
krystophny added a commit to computor-org/vllm-mlx that referenced this pull request Mar 26, 2026
Add test confirming deduplicated terminal blocks correctly isolate
recurrent state per sequence. Remove the duplicate tokenizer return fix
that already ships in PR waybarrios#215.
krystophny added a commit to computor-org/vllm-mlx that referenced this pull request Mar 26, 2026
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.

2 participants