Skip to content

fix(tests): align two stale tests in #1259 with current implementation#1287

Merged
jundot merged 1 commit into
jundot:mainfrom
richgoodson:fix/test-stale-mtp-eligibility-and-vlm-processor-stubs
May 19, 2026
Merged

fix(tests): align two stale tests in #1259 with current implementation#1287
jundot merged 1 commit into
jundot:mainfrom
richgoodson:fix/test-stale-mtp-eligibility-and-vlm-processor-stubs

Conversation

@richgoodson
Copy link
Copy Markdown
Contributor

Summary

Two of the failing tests called out in #1259 are stale against the
current implementation, not regressions in the code under test. Both
were already failing on main for me on a fresh editable install; this
PR brings them back in line with the code they're meant to cover.

Pairs with PR #1268 (test_admin_profiles_api) and #1286
(test_settings) to fully clear the #1259 list.

Changes

tests/test_mlx_lm_mtp_patch.py::TestBatchGeneratorDispatch::test_is_mtp_eligible_requires_mtp_forward_and_solo_batch

The test was written against the pre-#1247 contract where head presence
implied MTP active. Commit
23ca7dc decoupled head
attachment from inference-time MTP for the VLM load path and added an
is_mtp_active() gate inside _is_mtp_eligible. The True-expected
assertion now fails because the process-wide flag is off.

Fix: toggle mlx_lm_mtp.set_mtp_active(True) around the True-expected
assertions, restore the prior value in finally, and add a new
"head attached but flag off" case to lock in the post-23ca7dc
semantics (this is the exact case the VLM runtime patches rely on).

tests/test_vlm_torch_free_image_processor.py::test_patch_wraps_target_processors

The test stubbed the wrong module path and class name for dots_ocr:

test stubbed code imports
module mlx_vlm.models.dots_ocr.processing mlx_vlm.models.dots_ocr.processing_dots_ocr
class DotsOcrProcessor DotsVLProcessor

The fake_import branch never matched, the dots branch silently fell
through to the except (ImportError, AttributeError) clause inside
_patch_torch_free_image_processor, and the wrap-marker assertion
failed. The mismatch has been there since the test was added in
a1987ed.

Fix: align the stubs with the actual (module_path, cls_name) tuples
in omlx/engine/vlm.py:181-184.

Testing

Before:

$ pytest tests/test_mlx_lm_mtp_patch.py tests/test_vlm_torch_free_image_processor.py
... 2 failed, 52 passed

After:

$ pytest tests/test_mlx_lm_mtp_patch.py tests/test_vlm_torch_free_image_processor.py
... 54 passed in 0.21s

Apple Silicon M5, Python 3.13.12, fresh editable install on
51907f0.

Scope

Test-only changes. No code under omlx/ modified.

Refs #1259.

🤖 Generated with Claude Code

test_is_mtp_eligible_requires_mtp_forward_and_solo_batch was written
against a pre-jundot#1247 contract where head presence implied MTP active.
Commit 23ca7dc decoupled head attachment from inference-time MTP for
the VLM load path and added an is_mtp_active() gate. Set the flag
around the True-expected assertion, restore in finally, and add a new
"head attached but flag off" case to lock in the post-23ca7dc semantics.

test_patch_wraps_target_processors stubbed the wrong module path and
class name for dots_ocr (mlx_vlm.models.dots_ocr.processing /
DotsOcrProcessor), but the patcher imports
mlx_vlm.models.dots_ocr.processing_dots_ocr and looks up
DotsVLProcessor (per commit a1987ed, where the test was added). The
fake_import branch never matched, the dots branch silently fell
through to the except clause, and the wrap-marker assertion failed.
Align the test's stubs with the actual (module_path, cls_name) tuples
in vlm.py.

Both are test-only fixes. Refs jundot#1259.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
panwudi pushed a commit to panwudi/flyto-mlx that referenced this pull request May 18, 2026
Failures pre-date this branch (upstream issue jundot#1259 cluster) but were not
covered by upstream jundot#1268/jundot#1286/jundot#1287 because they track flyto's own
divergence:
- model_profiles: classify dflash_max_concurrent / dflash_kv_pressure_threshold
  (flyto dflash Path A fields, beyond upstream jundot#1268's 9)
- test_omlx_app: server_manager auto-restart cap was lifted 3 -> 10000
  (3bed072); update the constant assertion and the give-up test
- test_vlm_engine: _prepare_vision_inputs gained an audios kwarg

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jundot
Copy link
Copy Markdown
Owner

jundot commented May 19, 2026

Verified both fixes against current code. _is_mtp_eligible gates on is_mtp_active() at omlx/patches/mlx_lm_mtp/batch_generator.py:222-224 (added in 23ca7dc), and _patch_torch_free_image_processor does import processing_dots_ocr.DotsVLProcessor at omlx/engine/vlm.py:183. The new "head attached but flag off" case is a nice lock-in for the post-23ca7dc semantics. Merging.

@jundot jundot merged commit ad004a1 into jundot:main May 19, 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