Skip to content

fix: Reconcile the Qwen2-VL XLA loader contract with its failing capability test #966

Description

@inureyes

Problem / Background

multimodal::host_preprocessor::tests::xla_loader_keeps_text_and_unqualified_vlm_image_capability_false fails deterministically on main (14412c13f). The test pins a Qwen2-VL loader contract that #915 changed, and the commit that changed the contract edited the same test file without updating this test.

Reproduction:

DEVELOPER_DIR=/Applications/Xcode-26.6.0.app/Contents/Developer cargo test --release --lib --features metal,accelerate multimodal::host_preprocessor

Observed:

thread 'multimodal::host_preprocessor::tests::xla_loader_keeps_text_and_unqualified_vlm_image_capability_false' panicked at src/multimodal/host_preprocessor_tests.rs:77:74:
called `Result::unwrap()` on an `Err` value: InvalidConfig("Qwen2-VL XLA image execution requires the xla-iree feature")

test result: FAILED. 11 passed; 1 failed; 0 ignored; 0 measured; 4854 filtered out; finished in 0.06s

The two sides of the contract

Test, src/multimodal/host_preprocessor_tests.rs:69-83: loops over ["llama", "qwen2_vl"], writes a config.json containing only model_type into a tempdir, calls load_xla_image_preprocessor(model_dir.path()).unwrap(), and asserts the result is_none() with the message "is not a qualified LLaVA host/runtime pair".

Production, src/multimodal/host_preprocessor.rs:154-181: with model_type = qwen2_vl and the xla-iree feature off, the #[cfg(not(feature = "xla-iree"))] arm returns Err(HostPreprocessorError::InvalidConfig("Qwen2-VL XLA image execution requires the xla-iree feature")). The .unwrap() at line 77 then panics.

Attribution: git log -S'Qwen2-VL XLA image execution requires' -- src/multimodal/host_preprocessor.rs points at edc0ebbb6, "feat: add Qwen2-VL OpenXLA vision path (#915)". That commit did touch host_preprocessor_tests.rs (80 lines), but only to add the export_qwen2_vl_prefill import and two new Qwen2-VL export tests. The ["llama", "qwen2_vl"] loop was left untouched; git log -L 69,83:src/multimodal/host_preprocessor_tests.rs shows its last edit was #895. The contract and the test that pins it therefore diverged inside a single change to a single file.

Not caused by recent work on this tree. #961 (33671f599) only gated the export_qwen2_vl_prefill import and definition on any(feature = "xla-iree", test); at its parent b4b587e72 both the error arm and the test loop are identical to main, so the failure reproduces there too, and the #961 PR body already records this test as failing before and after and out of scope. It is also unrelated to #953, which repaired a different deterministic failure in src/models/mllama/text.rs.

Reading A: the production code is right and the test is stale

  • feat: add Qwen2-VL OpenXLA vision path (#865) #915 qualified Qwen2-VL for the XLA path, so the doc comment's Ok(None) rationale ("VLM families whose processor/position contract has not been qualified for XLA yet") no longer describes qwen2_vl. Feature absence is a different axis (build configuration) that the doc comment does not speak to.
  • Qwen2-VL has no host vision path at all. The arm immediately above returns an error for MLXCEL_XLA_VISION_BACKEND=host with "Qwen2-VL XLA vision has no MLX fallback". Ok(None) for such a checkpoint is indistinguishable from "text-only checkpoint", so a session would start with images silently unsupported.
  • Feature absence already errors elsewhere in the same function: src/multimodal/host_preprocessor.rs:234-239 returns Err with "MLXCEL_XLA_VISION_BACKEND=iree requires the xla-iree feature".
  • Nothing working is taken away by failing at load. xla-iree = ["xla-backend", "mlxcel-xla/iree"], and without mlxcel-xla/iree the session's prefill_first_token returns NOT_WIRED (src/lib/mlxcel-xla/src/lib.rs:445-455). An xla-backend-only build can select MLXCEL_BACKEND=xla but cannot prefill or decode anything.

Fix under this reading: drop qwen2_vl from the loop, or split it into its own test asserting Err(HostPreprocessorError::InvalidConfig(_)), and keep llama asserting Ok(None).

Reading B: the test is right and the code regressed

  • The doc comment on load_xla_image_preprocessor states that Ok(None) "is the conservative result for text-only checkpoints and VLM families whose processor/position contract has not been qualified for XLA yet", and reserves startup errors for a narrower case: "Once a checkpoint is identified as the supported LLaVA family, missing or malformed processor/projector weights are startup errors rather than a capability downgrade." A hard error for a family the binary was simply not built for is a third case the documented contract does not sanction.
  • Asymmetry inside the same function: without xla-iree, LLaVA does not error. Unless the policy is explicitly iree, it falls through to load_llava_host_preprocessor_boxed and returns Ok(Some(host)). Qwen2-VL is the only family where a missing compile-time feature is fatal.
  • Both callers turn Err into a hard startup failure with no degrade path: src/backend/xla.rs:110 maps it to "OpenXLA image preprocessor load failed" inside create_session, and src/server/batch/xla_preprocess.rs:74 reports it through ready_tx as a worker startup error. Under the current code a text-only prompt against a Qwen2-VL checkpoint on the XLA backend cannot even create a session.
  • The neighbouring load_llava_host_preprocessor_boxed maps FamilyMismatch to Ok(None) with the comment "Keep capability false for that combination", which is the precedent the test name is written against.

Fix under this reading: return Ok(None) when the feature is absent.

Which reading is better supported

Reading A, though not decisively, and the doc comment needs amending either way.

The only configuration in which the error is reachable is xla-backend without mlxcel-xla/iree, and in that configuration the XLA session is inert (NOT_WIRED on prefill). Downgrading capability to false there buys no working behavior and costs a specific diagnostic. Reading B's strongest point is the LLaVA asymmetry, but that asymmetry has a substantive cause rather than being an oversight: LLaVA has a real host vision implementation and Qwen2-VL does not, which is exactly what the adjacent "no MLX fallback" error says.

What would settle it, for whoever owns the OpenXLA vision work:

  1. Whether an xla-backend-without-xla-iree build is a configuration anyone is expected to point a Qwen2-VL checkpoint at, text-only or otherwise. If yes, Reading B; if that build is inert by design, Reading A.
  2. Whether any current or planned caller needs to distinguish "no image capability" from "misbuilt binary", for example a capability probe on /v1/models, or a server that should come up text-only instead of refusing to start.

The wider pattern

This is the third deterministically failing test found on main in one week. The other two were models::mllama::text::tests::ragged_real_tile_rows_match_reference_masked_full_rows (repaired by #953) and family_order_is_exhaustive in src/main_tests.rs (repaired by #946); the header of .github/workflows/nightly-verify.yml records an earlier instance repaired by #939. All of them survived because no CI job ran the general unit suite: PR CI runs fast checks only, and the pipeline-parallel workflow is path-filtered.

#953 added .github/workflows/nightly-verify.yml, which runs make verify (cargo fmt --check, clippy, and cargo test --release --features metal,accelerate) on the self-hosted Apple Silicon runner daily at 18:00 UTC. This issue is the first failure that nightly should catch on its own, and the nightly will report red until it is resolved.

Acceptance Criteria

  • cargo test --release --lib --features metal,accelerate multimodal::host_preprocessor is green (12 passed, 0 failed).
  • A decision is recorded in this issue or its PR on which contract load_xla_image_preprocessor holds for model_type = qwen2_vl when xla-iree is absent: Ok(None) capability downgrade, or Err(InvalidConfig) build-configuration error.
  • The chosen contract is documented on the load_xla_image_preprocessor doc comment alongside the existing Ok(None) and startup-error cases, so the feature-absent axis is no longer unstated.
  • A test pins whichever behavior is chosen for qwen2_vl, asserting on the concrete return value rather than only on is_none(). The llama case keeps an Ok(None) assertion either way.
  • If the resolution changes the production return value, both call sites (src/backend/xla.rs, src/server/batch/xla_preprocess.rs) are reviewed against the new value and the resulting startup behavior is stated in the PR.
  • cargo test --release --features metal,accelerate shows no other deterministic failure introduced by the change.
  • The next Nightly verify run is green for this test.

Technical Considerations

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:modelsModel architectures, weights, loading, metadatapriority:mediumMedium prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions