Skip to content

[nightly-verify] main is red #962

Description

@github-actions

The nightly make verify job added in #953 is red. This issue is filed by that job and is reused by subsequent nightly failures while it stays open, so the run table above the comments is the history, not a single snapshot. Reproduce locally with make verify (or the single failing target reported in the latest comment).

The body below is the diagnosis, so this can be picked up without opening the Actions runs.

Run history and what is already fixed

Run Commit verify-fmt verify-clippy verify-test
2026-07-28 b4b587e72 success failure failure
2026-07-29 b4b587e72 success failure failure
2026-07-31 ac793d675 success success failure

Two of the three original failures are already fixed on main, so neither is part of the remaining work.

The clippy half was fixed by #961. On b4b587e72 the failures were error: unused import: export_qwen2_vl_prefill and error: function export_qwen2_vl_prefill is never used, both promoted to errors by -D warnings. The function's only non-test call site sits inside a #[cfg(feature = "xla-iree")] block, so under the nightly's --features metal,accelerate (which does not enable xla-iree) the import and the definition both compiled out of use. #961 gated the import and the definition on any(feature = "xla-iree", test). verify-clippy has been green since.

The verify-test failure on b4b587e72 was a different test and was fixed by #987. It was the lib unit test multimodal::host_preprocessor::tests::xla_loader_keeps_text_and_unqualified_vlm_image_capability_false, panicking at src/multimodal/host_preprocessor_tests.rs:77, reported as 4722 passed; 1 failed; 113 ignored. #961's own PR body flagged that test as pre-existing and out of scope, and #987 fixed it. On ac793d675 the lib target passes, and on the current main (649f0a520) it reports 4813 passed; 0 failed; 113 ignored.

So the remaining work is the verify-test failure seen on ac793d675, and only that.

Remaining work: two independent defects, both in tests/cli_help_consistency.rs

cargo test is fail-fast by default, so the nightly stops at the first failing test target and the 70 other integration targets under tests/ never ran. Whoever picks this up should re-run with --no-fail-fast at least once after fixing the two below, because there may be more behind them.

Defect 1: the test helper ignores CARGO_TARGET_DIR, so all 11 tests in the file fail on CI

This is the failure the nightly actually reports, and it does not reproduce locally. On the 2026-07-31 run every test in tests/cli_help_consistency.rs failed, all with the same panic:

thread '...' panicked at tests/cli_help_consistency.rs:100:29:
failed to spawn mlxcel from "/Users/runner/actions-runner/_work/mlxcel/mlxcel/target/release/mlxcel": No such file or directory (os error 2)

Line 100 is the unwrap_or_else on Command::output() in help_output, not an assertion. The cause is repo_binary_path in tests/common/mod.rs: it reads CARGO_BIN_EXE_<name> with std::env::var_os (a compile-time variable that is not set at test runtime), then falls back to CARGO_MANIFEST_DIR/target/{debug,release}/<name>. That fallback hardcodes target/. The nightly workflow sets CARGO_TARGET_DIR=$HOME/.cargo-target/mlxcel in its "Setup persistent cache paths" step, so cargo builds the binaries into that directory while the test looks for them in the checkout. release.yml uses the same persistent path, so this is not a nightly-only quirk.

repo_binary_path is shared by 12 integration test files (chat_template_kwargs.rs, speculative_parity.rs, structured_outputs.rs, thinking_budget.rs, prompt_cache_e2e.rs, and others), so the fix is not local to the help test. Most of the others self-skip on a missing models/<name> directory and may not have reached the spawn, but that is unverified because of the fail-fast stop.

The fix should resolve the binary in a way that does not assume the target directory is <manifest>/target. Options: honor CARGO_TARGET_DIR explicitly, or derive the target directory from std::env::current_exe() (the test binary itself lives at <target>/<profile>/deps/), or use the compile-time env!("CARGO_BIN_EXE_mlxcel") for the two known binaries. Whichever is chosen, the helper should panic with a clear message naming both the resolved path and the target directory it derived, so the next occurrence is diagnosable from the panic alone.

Defect 2: drafter_flag_aliases_are_documented_on_both_binaries asserts on a string clap no longer emits

This is what fails locally, where target/release/ does hold the binaries. Running the built test binary directly gives 10 passed; 1 failed, and the one failure is:

thread 'drafter_flag_aliases_are_documented_on_both_binaries' panicked at tests/cli_help_consistency.rs:475:5:
mlxcel serve --help must document --draft-model with a --model-draft alias.

Narrow reproduce:

DEVELOPER_DIR=/Applications/Xcode-26.6.0.app/Contents/Developer cargo test --release --features metal,accelerate --test cli_help_consistency drafter_flag_aliases

The functionality is correct and the test's expected string is wrong. The test makes four assertions (starting at lines 475, 480, 485, 490), each pairing a flag spelling with the literal [aliases: X]. Both binaries render the singular form. clap_builder computes the label as pluralize(als.len(), "", "es") in src/output/help_template.rs, so it prints [alias: ...] for exactly one visible alias and [aliases: ...] only for two or more. Each of the four flags carries exactly one visible_alias, so all four render singular.

This is a dependency-bump regression, not a code change. clap_builder 4.6.0 emitted [aliases: ...] unconditionally; the pluralize call arrived in 4.6.2, which entered Cargo.lock on 2026-07-20 in 3ccb693a3 (the dependabot bump in #828). The test was written on 2026-07-01 in a89f28aa4 (#602) against the older rendering. The lock currently pins clap 4.6.4 / clap_builder 4.6.2.

Measured on the binaries built from 649f0a520:

  • mlxcel serve --help contains --draft-model <PATH> with [alias: --model-draft], and --draft-max <DRAFT_MAX> with [alias: --draft].
  • mlxcel-server --help contains --model-draft <PATH> with [alias: --draft-model], and --draft <DRAFT> with [alias: --draft-max].
  • Neither help output contains the substring [aliases: anywhere.

So the #464 contract the test exists to pin genuinely holds: each binary keeps the opposite primary spelling and both accept and document the other. src/main.rs declares visible_alias = "model-draft" on draft_model and visible_alias = "draft" on draft_max; src/bin/mlx_server.rs declares visible_alias = "draft-model" and visible_alias = "draft-max" (lines 273 and 282). The parse-equivalence half is separately covered by serve_draft_model_and_model_draft_aliases_resolve_identically and serve_draft_max_and_draft_aliases_resolve_identically in src/main_tests.rs, and by model_draft_and_draft_model_aliases_resolve_identically and draft_and_draft_max_aliases_resolve_identically in src/bin/mlx_server.rs. Those pass.

Verify independently by running both --help commands and grepping for alias.

Fix the test, not the CLI. Adding a second alias to each flag so clap emits the plural form would be the wrong direction: it changes the operator-facing CLI surface to satisfy a test string.

The replacement assertion should not re-break the next time clap changes its singular/plural rendering. Two candidate shapes: assert that the alias name appears in the help block for that flag, or accept either [alias: or [aliases:. Whoever fixes it should pick one and justify the choice in the PR, because an over-loose assertion stops pinning the #464 contract at all. The test comment at lines 460 to 469 already states what the contract is; keep that intent intact.

Acceptance Criteria

  • cargo test --release --features metal,accelerate --test cli_help_consistency is green.
  • The same command is green when run with CARGO_TARGET_DIR pointed at a directory outside the checkout, matching what the nightly workflow sets.
  • The fix(cli): align mlxcel-server drafter flags with mlxcel serve (--draft-model rejected) #464 contract is still pinned: a build that dropped any one of the four visible_alias declarations still fails the test. Demonstrate this, for example by removing one alias locally and showing the test goes red.
  • cargo test --release --features metal,accelerate --no-fail-fast is run at least once so any failures hidden behind the fail-fast stop are surfaced and either fixed or filed.
  • make verify is green end to end, so the next nightly closes this issue rather than reusing it.

Notes for whoever picks it up

make verify-test runs cargo test --release --features metal,accelerate from the repository root. The root Cargo.toml declares no default-members, so this is the root mlxcel package only (lib, bins, and all 71 integration targets under tests/), not the whole workspace; mlxcel-core, mlxcel-surgery, and mlxcel-xla tests are not included. The cost is dominated by linking those 71 integration binaries, which takes a long time on this machine. The narrow --test cli_help_consistency selector above is enough to iterate on.

Every cargo invocation on macOS needs DEVELOPER_DIR=/Applications/Xcode-26.6.0.app/Contents/Developer. Without it xcrun -f metal fails with xcrun: error: unable to find utility "metal", not a developer tool or in PATH and the cmake step of the mlxcel-core build script fails.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:highHigh 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