Skip to content

fix(loading): route Mistral3-VLM mistral4 (MLA) text backbone to the Mistral4 loader (#423)#424

Merged
inureyes merged 3 commits into
mainfrom
fix/issue-423-mistral4-vlm-text-backbone
Jun 24, 2026
Merged

fix(loading): route Mistral3-VLM mistral4 (MLA) text backbone to the Mistral4 loader (#423)#424
inureyes merged 3 commits into
mainfrom
fix/issue-423-mistral4-vlm-text-backbone

Conversation

@inureyes

Copy link
Copy Markdown
Member

Summary

Mistral3 VLM containers (Mistral3ForConditionalGeneration) can wrap a mistral4 (MLA + MoE) text backbone, but build_pixtral_family_context always loaded the text backbone through the Llama3 loader, which fails because an MLA backbone has no q_proj. This routes the mistral4 case to the Mistral4 loader, unblocking mlx-community/Mistral-Small-4-119B-2603-4bit.

Root cause

build_pixtral_family_context (src/loading/vlm_pixtral.rs) unconditionally parsed text_config as models::llama3::ModelArgs and built models::Llama3Model::from_weights, which expects standard attention weights (q_proj). For a mistral4 backbone the attention is MLA (q_a_proj / kv_a_proj_with_mqa / kv_b_proj, no q_proj), so loading failed with Weight not found: model.layers.0.self_attn.q_proj.weight. The text-only path load_llama_family_from_weights (src/loading/mod.rs) already branches on is_mistral4_config and routes to Mistral4Model::from_weights; the VLM context builder lacked the equivalent branch.

What changed

  • src/loading/vlm_pixtral.rs: in build_pixtral_family_context, branch the text-backbone load on is_mistral4_config(&full_config). The mistral4 branch parses the raw full_config["text_config"] (with the top-level quantization block inherited via inherit_quantization_if_missing when absent) as models::mistral4::Mistral4Config, runs models::mistral4::sanitize_weights(&mut weights, &args, ""), and builds models::Mistral4Model::from_weights(&weights, &args) wrapped as LoadedModel::Mistral4, mirroring the text-only load_llama_family_from_weights path.
  • The raw text_config is used for model construction rather than the Llama-massaged value from build_mistral_text_config, because infer_llama_config_from_weights and apply_mistral_attention_head_override key off q_proj and can corrupt the MLA config.
  • Weights are already stripped of the language_model. prefix by strip_language_model_prefix before this point, so both loaders see the bare model.layers... namespace.
  • The Llama path is unchanged for non-mistral4 Mistral3 VLMs (Pixtral and standard Mistral text backbones). text_config_value is still built via build_mistral_text_config and used only for the scalar context fields (hidden_size, rms_norm_eps), which survive the Llama massaging on the mistral4 path.

Test plan

  • cargo fmt --check
  • cargo check --lib --features metal,accelerate
  • cargo clippy --lib --tests --features metal,accelerate -- -D warnings
  • cargo test --release --features metal,accelerate --lib loading::tests::is_mistral4 (2 passed)

A checkpoint-free unit test of the private routing branch is not practical (it needs a real model directory plus full model construction); the existing is_mistral4_config detection tests cover the predicate. Real-model load validation of mlx-community/Mistral-Small-4-119B-2603-4bit is performed by the orchestrator.

Closes #423

…Mistral4 loader (#423)

build_pixtral_family_context unconditionally parsed the VLM text backbone as models::llama3::ModelArgs and loaded it via Llama3Model::from_weights, which expects standard attention weights (q_proj). A Mistral3ForConditionalGeneration checkpoint whose text_config.model_type is mistral4 has an MLA + MoE backbone (q_a_proj / kv_a_proj_with_mqa / kv_b_proj, no q_proj), so the load failed with "Weight not found: model.layers.0.self_attn.q_proj.weight". This blocked loading mlx-community/Mistral-Small-4-119B-2603-4bit, the only public mistral4 checkpoint, which ships only as a VLM.

Branch the text-backbone load on is_mistral4_config(&full_config), mirroring the text-only load_llama_family_from_weights path in src/loading/mod.rs. On the mistral4 branch, parse the raw text_config (with the top-level quantization block inherited when absent) as models::mistral4::Mistral4Config, run models::mistral4::sanitize_weights, and build models::Mistral4Model::from_weights wrapped as LoadedModel::Mistral4. The raw text_config is used rather than the Llama-massaged value because infer_llama_config_from_weights and apply_mistral_attention_head_override key off q_proj and can corrupt the MLA config. Weights are already stripped of the language_model. prefix by strip_language_model_prefix, so both loaders see the bare model.layers... namespace.

The Llama path is unchanged for non-mistral4 Mistral3 VLMs (Pixtral and standard Mistral text backbones). text_config_value is still built via build_mistral_text_config for the scalar context fields (hidden_size, rms_norm_eps), which survive the Llama massaging on the mistral4 path. Real-model load validation of Mistral-Small-4-119B-2603-4bit is performed by the orchestrator.
@inureyes inureyes added status:review Under review type:bug Bug fixes, error corrections, or issue resolutions priority:medium Medium priority area:models Model architectures, weights, loading, metadata area:inference Generation, sampling, decoding (incl. speculative, DRY) labels Jun 24, 2026
@inureyes

Copy link
Copy Markdown
Member Author

Real-model validation (Mistral-Small-4-119B-2603-4bit)

Before this fix, loading the Mistral Small 4 VLM checkpoint aborted with Failed to load text model: Weight not found: model.layers.0.self_attn.q_proj.weight (the text backbone is mistral4 MLA: q_a_proj / kv_a_proj_with_mqa / kv_b_proj, no q_proj).

With this fix, mlxcel-server -m models/Mistral-Small-4-119B-2603-4bit loads in ~22s and serves coherent text generation. Prompt "Explain in two sentences why the sky appears blue during the day." (temperature 0) returns: "The sky appears blue due to Rayleigh scattering, where shorter (blue) wavelengths of sunlight are scattered more by the molecules in the atmosphere than other colors." Non-mistral4 Mistral3 VLMs are unaffected (the Llama text-backbone path is unchanged).

@inureyes

Copy link
Copy Markdown
Member Author

Implementation Review Summary

Intent

Route a Mistral3-VLM container whose text backbone is mistral4 (MLA + MoE, no q_proj) to the Mistral4 loader instead of the Llama3 loader, unblocking mlx-community/Mistral-Small-4-119B-2603-4bit.

Findings Addressed

None. The implementation is correct and complete as submitted; no fixes were required.

Verification

  • All stated requirements implemented — mistral4 branch added to build_pixtral_family_context, mirrors the text-only load_llama_family_from_weights path (raw text_config, sanitize_weights(.., ""), Mistral4Model::from_weights).
  • No placeholder/mock code remaining.
  • Integrated into project code flow — LoadedModel::Mistral4 is boxed into VisionLanguageModel.text_model identically to LoadedModel::Llama; both VLM entry points (load_pixtral_vlm, load_mistral3_vlm) route through the shared context builder.
  • Project conventions followed — is_mistral4_config imported from crate::model_metadata (same path as mod.rs); upstream issue ref qualified as lablup/mlxcel#423.
  • Existing modules reused — is_mistral4_config, inherit_quantization_if_missing, models::mistral4::sanitize_weights, Mistral4Model::from_weights.
  • No unintended structural changes — Llama (non-mistral4) path is functionally unchanged for Pixtral / standard Mistral3 VLMs. text_config_value still feeds only the scalar context fields (hidden_size, rms_norm_eps); infer_llama_config_from_weights only fills missing fields and never touches rms_norm_eps, and apply_mistral_attention_head_override is a no-op without q_proj, so both stay correct on the mistral4 path.
  • Tests pass — cargo check --lib, cargo clippy --lib --tests -- -D warnings (clean), cargo test loading::tests::is_mistral4 (2 passed). Real-model load validated separately by the orchestrator.

Closes #423 is present in the PR body.

Mistral4Model overrode forward_with_embeddings but not embed_tokens, so it fell through to the LanguageModel trait default that returns None. PR #424 is the first to route a mistral4 (MLA + MoE) backbone into a Mistral3 VLM container, making it the first caller of embed_tokens on this model: VisionModule::get_input_embeddings (src/vision/mod.rs) calls it to build text-space embeddings before merging projected image features. With the default None, any image request to a Mistral4-backed Mistral3 VLM failed with "Text model must support embed_tokens for VLM". Text-only generation was unaffected because that path uses forward(input_ids) directly.

Override embed_tokens to return the same self.embed_tokens.forward(input_ids) used by the no-input_embeddings branch of forward_with_embeddings and by the plain forward path, so the merge path and the forward path share one embedding table. Mirrors the llama3 backbone (src/models/llama3.rs).
@inureyes

Copy link
Copy Markdown
Member Author

Follow-up: fixed Mistral4 VLM image-path gap (HIGH)

Deeper review found one issue that the text-only load validation could not surface, and it is now fixed on this branch (commit 29340f0).

Finding (HIGH): Mistral4-backed Mistral3 VLM image requests fail at runtime. Mistral4Model (src/models/mistral4.rs) implements forward_with_embeddings but did not override LanguageModel::embed_tokens, so it fell through to the trait default that returns None (mlxcel-core/src/generate.rs:308). This PR is the first to route a mistral4 backbone into a Mistral3 VLM container, which makes it the first caller of embed_tokens on this model: VisionModule::get_input_embeddings (src/vision/mod.rs:176-177) calls it to build text-space embeddings before merging projected image features, and errors with Text model must support embed_tokens for VLM when it returns None. Text-only generation was unaffected (that path goes through forward(input_ids) directly), which is why the load/text validation passed.

Fix: override embed_tokens on Mistral4Model to return Some(self.embed_tokens.forward(input_ids)), the same embedding lookup used by the no-input_embeddings branch of forward_with_embeddings and by the plain forward path. This mirrors the llama3 backbone (src/models/llama3.rs:857) and keeps the merge path and the forward path on one embedding table. This completes the multimodal (image+text) acceptance criterion from #423, not just the text-only path.

Verification (narrow):

  • cargo fmt --check clean
  • cargo check --lib --features metal,accelerate clean
  • cargo clippy --lib --tests --features metal,accelerate -- -D warnings clean
  • cargo test --release --features metal,accelerate --lib loading::tests::is_mistral4 2 passed

An image+text request against Mistral-Small-4-119B-2603-4bit should be re-validated by the orchestrator to confirm the multimodal path end to end; the routing and embedding plumbing are now in place.

@inureyes

Copy link
Copy Markdown
Member Author

Correction on the image+text path (text-only is validated; image+text is NOT yet working)

The embed_tokens override is a necessary prerequisite but does NOT complete the image+text path. Re-validating multimodal on the real 119B after that fix, text-only generation works (loads ~22s, coherent), but an image+text request still aborts during multimodal prefill:

libc++abi: terminating due to uncaught exception of type std::invalid_argument: [reshape] Cannot reshape array of size 12480512 into shape (1,1,1).

That size is 3047 x 4096 (merged text+image embeddings at hidden_size 4096), so the crash is in the mistral4 multimodal-prefill path (Llama-4 attention-scale / position handling with image embeddings), a further new-backbone-in-VLM wiring gap distinct from the loader routing and embed_tokens.

Scope of this PR: it satisfies #423's required acceptance criteria (the checkpoint loads without the q_proj error and serves coherent text generation, which previously aborted at load). #423's image+text is an explicit bonus criterion and is deferred to a follow-up issue; the embed_tokens override is retained as the correct first step toward it. Text-only generation and non-mistral4 Mistral3 VLMs are unaffected.

… supported-models

The Mistral 3 VLM entry in supported-models.md now records that Mistral3 VLM routes Mistral4 (MLA+MoE) text backbones via the Mistral4 loader, that text generation is validated, and that image+text for the Mistral4 backbone is tracked in #425.
@inureyes

Copy link
Copy Markdown
Member Author

PR Finalization

Tests

Existing coverage is complete for the checkpoint-free scope. The detection predicate and routing policy are covered by four tests in src/loading/tests.rs (is_mistral4_config_detects_nested_model_type, is_mistral4_config_returns_false_without_matching_text_model, directory_load_route_handles_mistral3_mistral4_subtype, model_load_policy_handles_mistral3_mistral4_wrapper_config). The helper functions used in the new VLM branch (inherit_quantization_if_missing, build_mistral_text_config) are covered in src/loading/vlm_pixtral_tests.rs. The build_pixtral_family_context routing and embed_tokens override both require real weights and are out of reach for unit tests; no synthetic test was forced.

Documentation

Updated the "Pixtral and Mistral 3 VLM wrappers" entry in docs/supported-models.md to record that Mistral 3 VLM now routes Mistral4 (MLA+MoE) text backbones via the Mistral4 loader, that text generation is validated, and that image+text is tracked in #425.

Code quality

  • cargo fmt --check: clean
  • cargo clippy --lib --tests -- -D warnings: clean
  • cargo check --lib: clean

All checks passing. Not merged.

@inureyes inureyes added status:done Completed and removed status:review Under review labels Jun 24, 2026
@inureyes
inureyes merged commit 12389da into main Jun 24, 2026
5 checks passed
@inureyes
inureyes deleted the fix/issue-423-mistral4-vlm-text-backbone branch June 24, 2026 15:34
inureyes added a commit that referenced this pull request Jun 24, 2026
…ng (#425) (#426)

* fix(models): flatten mistral4 MoE tokens to 2D before SwitchGLU routing (#425)

Mistral4MoE::forward passed a 3D `[batch, seq, hidden]` tensor and 3D `[batch, seq, top_k]` routing indices to the shared SwitchGLU::forward, which expects 2D `[n_tokens, hidden]` and `[n_tokens, top_k]`. SwitchGLU reads `indices_shape[0]` as `n_tokens` and `indices_shape[1]` as `top_k`, then decides `do_sort = n_tokens * top_k >= 64`. With 3D `[1, seq, top_k]` it misread `n_tokens=1` and `top_k=seq`. For `seq < 64` the `do_sort=false` branch limped along; for `seq >= 64` it entered gather_sort with the wrong dims and aborted with `[reshape] Cannot reshape array of size seq*hidden into (1,1,1)`.

This is the 3D-vs-2D SwitchGLU contract violation behind #425: it fired on every real mistral4 prompt, on a 512-token chunked-prefill continuation chunk, and on any image-bearing multimodal prefill (e.g. the 3047-token merged text+image sequence, 3047 x 4096 = 12480512). It stayed latent because mistral4 was only ever run on a single-token warmup before #424 made the family loadable; the warmup's `[1, 1, hidden]` MoE forward succeeds.

Fix mirrors qwen3_moe's proven flatten/reshape-back pattern: capture `orig_shape`, flatten `x` to `[n_tokens, hidden]` (prefill `[1, l, hidden]` -> `[l, hidden]`, decode `[1, 1, hidden]` -> `[1, hidden]`), run the existing routing math on the flat tensor so indices and scores come out correctly shaped `[n_tokens, top_k]`, dispatch experts and shared experts on the flat tensor, then reshape the result back to the original rank. Ops and eps are unchanged, so per-token routing is numerically identical, just correctly shaped. SwitchGLU, moe_weighted_sum, and all other models are untouched.

This also corrects the decode path, which the old 3D shape was misrouting. Decode output may differ slightly from the pre-fix buggy path; that is the intended correction, not a regression.

Validation: cargo fmt --check, cargo check --lib, and cargo clippy --lib --tests -- -D warnings all pass. Real-model 119B validation (chunked-prefill text plus image+text on mlx-community/Mistral-Small-4-119B-2603-4bit) is performed by the orchestrator.

* docs(models): document Mistral 4 (Mistral Small 4) text and vision support (#425)

Add a dedicated supported-models entry for the Mistral 4 (`mistral4`) MLA + MoE family now that it loads and runs end to end: the Mistral Small 4 119B checkpoint generates text and image-plus-text via the Mistral 3 VLM wrapper on the Mistral4 text backbone. Record the architecture values (q_lora_rank 1024, kv_lora_rank 256, split rope/nope head dims, 128 routed experts plus one shared with 4 active, Llama-4 attention scaling) and the validated checkpoint, and update the VLM wrapper note to reflect that image-plus-text on the Mistral4 backbone is now validated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:inference Generation, sampling, decoding (incl. speculative, DRY) area:models Model architectures, weights, loading, metadata priority:medium Medium priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(loading): Mistral3-VLM with mistral4 (MLA) text backbone fails to load (routes to Llama q_proj loader)

1 participant