Skip to content

fix: teach check_mlx_vlm.py the >= floor certifi uses (#278 regression) - #283

Merged
khosravipasha merged 1 commit into
PrismML-Eng:mainfrom
xiaods:fix/mlx-vlm-checker-certifi
Nov 2, 2026
Merged

khosravipasha merged 1 commit into
PrismML-Eng:mainfrom
xiaods:fix/mlx-vlm-checker-certifi

Conversation

@xiaods

@xiaods xiaods commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Problem

./scripts/run_mlx.sh -p "What is the capital of France?"

crashes before any model code runs:

Traceback (most recent call last):
  File ".../scripts/check_mlx_vlm.py", line 9, in <module>
    package, expected = requirement.split("==")
ValueError: not enough values to unpack (expected 2, got 1)

Cause

#278 added certifi>=2026.10.23 to scripts/requirements-mlx-vlm.txt as a security floor for the CA bundle (GHSA-6q48-6c7w-9gw2). check_mlx_vlm.py split every requirement on == only, so the >= line raised ValueError. run_mlx.sh calls this checker via require_bonsai2_mlx_vlm before touching the model, so every Bonsai 2 MLX launch on an existing .venv-vlm died there. The pre-existing unit tests in tests/test_native_mlx_versions.py regressed too (all errored on the same unpack).

Fix

Keep certifi a floor (newer bundles are fine — that is the point of the advisory) and make the checker speak both specifiers instead of pinning certifi:

  • scripts/check_mlx_vlm.py parses == pins and >= floors; floors compare numerically and accept any newer version, pins still require an exact string match. Unsupported specifiers exit with a clear message rather than an unpacking traceback, so future edits to the requirements file fail loudly.
  • tests/test_native_mlx_versions.py: mock certifi in every scenario and add cases for newer-floor pass, stale-floor rejection, and unsupported-specifier reporting.

Verification

  • tests/test_native_mlx_versions.py — 5/5 pass (previously 3 errors).
  • tests/test_mlx_setup_paths.py (6) and tests/test_seed_openwebui.py (5) pass.
  • Real venv: .venv-vlm/bin/python scripts/check_mlx_vlm.py passes.
  • End to end: ./scripts/run_mlx.sh -p "What is the capital of France?" loads the model and answers correctly.

…regression)

Commit 5037237 added 'certifi>=2026.10.23' to requirements-mlx-vlm.txt as a
security floor for the CA bundle (GHSA-6q48-6c7w-9gw2), but check_mlx_vlm.py
split every line on '==' only, so ./scripts/run_mlx.sh crashed with
ValueError: not enough values to unpack before any model code ran.

Support '==' pins and '>=' floors: floors compare numerically and accept any
newer version; unsupported specifiers exit with a clear message instead of an
unpacking traceback so future edits to the requirements file fail loudly.

Update tests: certifi is mocked in every scenario, and add cases for
newer-floor pass, stale-floor rejection, and unsupported specifier.

@khosravipasha khosravipasha 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.

🤖 Reviewing on behalf of the maintainers

Thanks for catching and fixing this startup regression! Reviewed the changes and ran all five focused checker tests successfully. This preserves the exact pins while allowing newer certifi releases, and looks ready to merge.

One minor follow-up: _version_key is suitable for the current date-based certifi versions, but it does not implement general Python version ordering for prerelease suffixes. If we add floors for other packages later, we should use a standard version parser. No need to expand this fix for that now.

My validation covered the checker tests, not a full model launch. Thanks again!

@khosravipasha
khosravipasha merged commit 1ebbdb2 into PrismML-Eng:main Nov 2, 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