Skip to content

fix(qwen4-exp): keep text inference on mlx-vlm path - #741

Merged
waybarrios merged 1 commit into
waybarrios:mainfrom
Thump604:604/qwen4-exp-vlm-text-path
Sep 3, 2026
Merged

waybarrios merged 1 commit into
waybarrios:mainfrom
Thump604:604/qwen4-exp-vlm-text-path

Conversation

@Thump604

Copy link
Copy Markdown
Collaborator

Qwen4-Exp is not compatible with the generic Qwen3.5 TextModel fallback. Keep text requests on the loaded mlx-vlm language model instead of constructing a mechanically compatible model that produces incorrect logits.

Checks:

  • 13 focused text-model dispatch tests
  • 954 non-integration Apple-Silicon tests
  • Ruff and Black

@janhilgard janhilgard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Correct fix for the right reason. Returning None so SimpleEngine stays on the loaded mlx-vlm model is better than the alternative here: a mechanically compatible skeleton that loads cleanly and emits wrong logits is far harder to diagnose than a model that refuses the fast path, because nothing fails — the output is just quietly worse.

Two details I checked rather than assumed:

  • str.startswith accepts a tuple, so _VLM_ONLY_TEXT_MODEL_PREFIXES works as written and covers qwen4_exp_text from the single entry — the parametrised test pins both spellings, which is what makes the prefix form safe to extend later
  • the check sits before _import_text_model_classes, so an unregistered architecture short-circuits instead of falling through to the qwen3_5 default that #686 made prefix-based

The logger.info naming the model type matters more than it looks: this path silently changes which engine serves text, and without that line the only symptom would be a throughput difference nobody attributes to dispatch.

Approving.

@waybarrios

Copy link
Copy Markdown
Owner

all set and ready to go

@waybarrios
waybarrios merged commit 761abfe into waybarrios:main Sep 3, 2026
10 checks passed
TimotejLabsky added a commit to TimotejLabsky/vllm-mlx that referenced this pull request Sep 23, 2026
…aybarrios#740 implicit-<think> on latch sites, system-KV index pin

Rebase onto waybarrios/vllm-mlx ec8e493 (11 commits past 22efb47): 172
fork commits replayed, 0 dropped, 4 conflict stops. Outside the four
resolutions the rebase's net delta matches upstream's window
hunk-for-hunk, so this commit carries only what the replay could not:

- waybarrios#741 (761abfe): re-add the qwen4_exp VLM-only text-path guard, lost
  when the text_model_from_vlm.py stops were resolved to the fork's
  candidate-chain dispatch. Load-bearing for waybarrios#97 once mlx-lm ships a
  qwen4_exp module (resident PLE table instead of the SSD-backed one).
- waybarrios#740 (d843a76): the Responses/Anthropic streaming latch sites (#27/waybarrios#47)
  build their reasoning parser directly, so they now pass implicit_mode
  from _detect_implicit_thinking, gated on thinking ON like upstream.
  GLM-4.7-Flash opens <think> in its generation prompt, so this changes
  that route's streamed reasoning split. One test stub updated to the
  new reset_state signature.
- waybarrios#740: SSDIndex._SCHEMA_VERSION 1 -> 2 would purge every system-KV SSD
  spill on first start although our entries don't use its serializers.
  Pin the system-KV store's index at 1 via _SystemKVIndex.
- waybarrios#729 (d80db21) partially retires patch waybarrios#95 (bench_command site stays).

PATCHES.md rebase note + README base pin updated. Suite 3601 passed /
31 skipped / 30 deselected; ruff clean. New tests mutation-checked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TimotejLabsky added a commit to TimotejLabsky/vllm-mlx that referenced this pull request Sep 25, 2026
…aybarrios#740 implicit-<think> on latch sites, system-KV index pin

Rebase onto waybarrios/vllm-mlx ec8e493 (11 commits past 22efb47): 172
fork commits replayed, 0 dropped, 4 conflict stops. Outside the four
resolutions the rebase's net delta matches upstream's window
hunk-for-hunk, so this commit carries only what the replay could not:

- waybarrios#741 (761abfe): re-add the qwen4_exp VLM-only text-path guard, lost
  when the text_model_from_vlm.py stops were resolved to the fork's
  candidate-chain dispatch. Load-bearing for waybarrios#97 once mlx-lm ships a
  qwen4_exp module (resident PLE table instead of the SSD-backed one).
- waybarrios#740 (d843a76): the Responses/Anthropic streaming latch sites (#27/waybarrios#47)
  build their reasoning parser directly, so they now pass implicit_mode
  from _detect_implicit_thinking, gated on thinking ON like upstream.
  GLM-4.7-Flash opens <think> in its generation prompt, so this changes
  that route's streamed reasoning split. One test stub updated to the
  new reset_state signature.
- waybarrios#740: SSDIndex._SCHEMA_VERSION 1 -> 2 would purge every system-KV SSD
  spill on first start although our entries don't use its serializers.
  Pin the system-KV store's index at 1 via _SystemKVIndex.
- waybarrios#729 (d80db21) partially retires patch waybarrios#95 (bench_command site stays).

PATCHES.md rebase note + README base pin updated. Suite 3601 passed /
31 skipped / 30 deselected; ruff clean. New tests mutation-checked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

3 participants